gollvm: sync up with recent gofrontend enhancements
This patch syncs up the gollvm bridge code with recent changes
in gofrontend, including
* new "decl_var" parameter in Backend::local_variable method
* debug_escape_hash Gogo parameter
For the LLVM bridge version of If Backend::local_variable, if a
non-NULL decl var is provided, we check to make sure that the variable
is part of the same function, then arrange for the new variable to
share the old variable's alloca instruction (which should give us the
desired behavior). This patch also changes lifetime start/end
annotation insertion avoid adding lifetimes for a declvar variable,
since its extent is based not on its enclosing block but on the var
whose alloca it shares.
Change-Id: I8e9825968ffb16d8ed1700f0395b44992834c9f0
Reviewed-on: https://go-review.googlesource.com/87436
Reviewed-by: Cherry Zhang <cherryyz@google.com>
diff --git a/bridge/go-c.h b/bridge/go-c.h
index 74a3f58..ce35693 100644
--- a/bridge/go-c.h
+++ b/bridge/go-c.h
@@ -47,6 +47,7 @@
bool check_divide_overflow;
bool compiling_runtime;
int debug_escape_level;
+ const char* debug_escape_hash;
int64_t nil_check_size_threshold;
};
diff --git a/bridge/go-llvm-bfunction.cpp b/bridge/go-llvm-bfunction.cpp
index fef36b7..dc24910 100644
--- a/bridge/go-llvm-bfunction.cpp
+++ b/bridge/go-llvm-bfunction.cpp
@@ -228,17 +228,36 @@
}
Bvariable *Bfunction::localVariable(const std::string &name,
- Btype *btype,
- bool is_address_taken,
- Location location)
+ Btype *btype,
+ Bvariable *declVar,
+ bool is_address_taken,
+ Location location)
{
lazyAbiSetup();
- llvm::Instruction *inst = addAlloca(btype->type(), name);
+ llvm::Instruction *inst = nullptr;
+ if (declVar != nullptr) {
+ // If provided, declVar must be an existing local variable in
+ // the same function (presumably at an outer scope).
+ assert(valueVarMap_.find(declVar->value()) != valueVarMap_.end());
+
+ // For the correct semantics, we need the two variables in question
+ // to share the same alloca instruction.
+ inst = llvm::cast<llvm::Instruction>(declVar->value());
+ } else {
+ inst = addAlloca(btype->type(), name);
+ }
Bvariable *bv =
new Bvariable(btype, location, name, LocalVar, is_address_taken, inst);
localVariables_.push_back(bv);
- assert(valueVarMap_.find(bv->value()) == valueVarMap_.end());
- valueVarMap_[bv->value()] = bv;
+ if (declVar != nullptr) {
+ // Don't add the variable in question to the value var map.
+ // Mark it so that it can be handled properly during creation
+ // of lifetime annotations.
+ bv->markAsDeclVar();
+ } else {
+ assert(valueVarMap_.find(bv->value()) == valueVarMap_.end());
+ valueVarMap_[bv->value()] = bv;
+ }
return bv;
}
diff --git a/bridge/go-llvm-bfunction.h b/bridge/go-llvm-bfunction.h
index ecc2d43..97f648a 100644
--- a/bridge/go-llvm-bfunction.h
+++ b/bridge/go-llvm-bfunction.h
@@ -66,15 +66,16 @@
// Add a local variable
Bvariable *localVariable(const std::string &name,
- Btype *btype,
- bool is_address_taken,
- Location location);
+ Btype *btype,
+ Bvariable *decl_var,
+ bool is_address_taken,
+ Location location);
// Add a parameter variable
Bvariable *parameterVariable(const std::string &name,
- Btype *btype,
- bool is_address_taken,
- Location location);
+ Btype *btype,
+ bool is_address_taken,
+ Location location);
// Create a Bvariable for the static chain param of the function.
Bvariable *staticChainVariable(const std::string &name,
diff --git a/bridge/go-llvm-bvariable.cpp b/bridge/go-llvm-bvariable.cpp
index 03625a3..43dc559 100644
--- a/bridge/go-llvm-bvariable.cpp
+++ b/bridge/go-llvm-bvariable.cpp
@@ -23,7 +23,7 @@
: name_(name), value_(value), initializer_(nullptr),
type_(type), underlyingType_(nullptr),
location_(location), which_(which),
- addrtaken_(address_taken), temporary_(false)
+ addrtaken_(address_taken), temporary_(false), declvar_(false)
{
}
@@ -33,7 +33,7 @@
: name_(name), value_(value), initializer_(nullptr),
type_(zeroSizeType), underlyingType_(underlyingNonZeroSizeType),
location_(location), which_(which),
- addrtaken_(address_taken), temporary_(false)
+ addrtaken_(address_taken), temporary_(false), declvar_(false)
{
assert(which == GlobalVar);
}
diff --git a/bridge/go-llvm-bvariable.h b/bridge/go-llvm-bvariable.h
index bbe0d8d..d826637 100644
--- a/bridge/go-llvm-bvariable.h
+++ b/bridge/go-llvm-bvariable.h
@@ -55,6 +55,8 @@
WhichVar flavor() const { return which_; }
bool isTemporary() const { return temporary_; }
void markAsTemporary() { temporary_ = true; }
+ bool isDeclVar() const { return declvar_; }
+ void markAsDeclVar() { declvar_ = true; }
// Set/get variable initializer. Some variables may not have an
// initializer, for example module-scoped globals, or variables
@@ -87,6 +89,7 @@
WhichVar which_;
bool addrtaken_;
bool temporary_;
+ bool declvar_;
friend class Llvm_backend;
};
diff --git a/bridge/go-llvm.cpp b/bridge/go-llvm.cpp
index 553c75c..9020d04 100644
--- a/bridge/go-llvm.cpp
+++ b/bridge/go-llvm.cpp
@@ -1751,7 +1751,7 @@
std::string tname(namegen("finvar"));
Bvariable *tvar = func->localVariable(tname, boolType(),
- false, location);
+ nullptr, false, location);
tvar->markAsTemporary();
Bstatement *excepst = nbuilder_.mkExcepStmt(func, bstat,
except_stmt,
@@ -2089,13 +2089,15 @@
Bvariable *Llvm_backend::local_variable(Bfunction *function,
const std::string &name,
Btype *btype,
+ Bvariable *decl_var,
bool is_address_taken,
Location location)
{
assert(function);
if (btype == errorType() || function == errorFunction_.get())
return errorVariable_.get();
- return function->localVariable(name, btype, is_address_taken, location);
+ return function->localVariable(name, btype, decl_var,
+ is_address_taken, location);
}
// Make a function parameter variable.
@@ -2137,7 +2139,7 @@
if (binit == errorExpression())
return errorVariable_.get();
std::string tname(namegen("tmpv"));
- Bvariable *tvar = local_variable(function, tname, btype,
+ Bvariable *tvar = local_variable(function, tname, btype, nullptr,
is_address_taken, location);
if (tvar == errorVariable_.get()) {
*pstatement = errorStatement();
@@ -2727,6 +2729,13 @@
{
assert(llbb);
for (auto v : block->vars()) {
+ if (v->isDeclVar()) {
+ // This variable does not have its own alloca -- it is borrowing the
+ // alloca instruction from some other outer-scope var, hence we
+ // don't have as much information about its lifetime: avoid inserting
+ // lifetime ops.
+ continue;
+ }
llvm::Value *ai = v->value();
appendLifetimeIntrinsic(ai, insertBefore, llbb, isStart);
}
diff --git a/bridge/go-llvm.h b/bridge/go-llvm.h
index fcbfbde..7b965d3 100644
--- a/bridge/go-llvm.h
+++ b/bridge/go-llvm.h
@@ -262,8 +262,8 @@
void global_variable_set_init(Bvariable *, Bexpression *);
- Bvariable *local_variable(Bfunction *, const std::string &, Btype *, bool,
- Location);
+ Bvariable *local_variable(Bfunction *, const std::string &, Btype *,
+ Bvariable *, bool, Location);
Bvariable *parameter_variable(Bfunction *, const std::string &, Btype *, bool,
Location);
diff --git a/driver/goparse-llvm.cpp b/driver/goparse-llvm.cpp
index 8e9f84b..13cf52b 100644
--- a/driver/goparse-llvm.cpp
+++ b/driver/goparse-llvm.cpp
@@ -164,6 +164,15 @@
"-fgo-optimize-allocs."),
cl::init(0));
+static cl::opt<std::string>
+EscapeDebugHash("fgo-debug-escape-hash",
+ cl::desc("A hash value to debug escape analysis. Argument is "
+ "a binary string. This runs escape analysis only on "
+ "functions whose names hash to values that match the "
+ "given suffix. Can be used to binary search across "
+ "functions to uncover escape analysis bugs."),
+ cl::init(""));
+
static cl::opt<unsigned>
TraceLevel("tracelevel",
cl::desc("Set debug trace level (def: 0, no trace output)."),
@@ -383,6 +392,9 @@
args.check_divide_overflow = CheckDivideOverflow;
args.compiling_runtime = CompilingRuntime;
args.debug_escape_level = EscapeDebugLevel;
+ args.debug_escape_hash = nullptr;
+ if (!EscapeDebugHash.empty())
+ args.debug_escape_hash = EscapeDebugHash.c_str();
args.nil_check_size_threshold = -1;
args.linemap = linemap_.get();
args.backend = bridge_.get();
diff --git a/unittests/BackendCore/BackendDebugEmit.cpp b/unittests/BackendCore/BackendDebugEmit.cpp
index 96fbaa8..d45c3e3 100644
--- a/unittests/BackendCore/BackendDebugEmit.cpp
+++ b/unittests/BackendCore/BackendDebugEmit.cpp
@@ -116,9 +116,9 @@
Location loc;
std::vector<Bvariable *> vlist;
- vlist.push_back(be->local_variable(func, "n1", set, false, loc));
- vlist.push_back(be->local_variable(func, "n2", beat, false, loc));
- vlist.push_back(be->local_variable(func, "n3", bi32t, false, loc));
+ vlist.push_back(be->local_variable(func, "n1", set, nullptr, false, loc));
+ vlist.push_back(be->local_variable(func, "n2", beat, nullptr, false, loc));
+ vlist.push_back(be->local_variable(func, "n3", bi32t, nullptr, false, loc));
h.newBlock(&vlist);
Bexpression *fn2 = be->function_code_expression(func2, loc);
diff --git a/unittests/BackendCore/BackendStmtTests.cpp b/unittests/BackendCore/BackendStmtTests.cpp
index ce66118..057c165 100644
--- a/unittests/BackendCore/BackendStmtTests.cpp
+++ b/unittests/BackendCore/BackendStmtTests.cpp
@@ -26,14 +26,14 @@
Location loc;
// local variable with init
- Bvariable *loc1 = be->local_variable(func, "loc1", bi64t, true, loc);
+ Bvariable *loc1 = be->local_variable(func, "loc1", bi64t, nullptr, true, loc);
Bstatement *is = be->init_statement(func, loc1, mkInt64Const(be, 10));
ASSERT_TRUE(is != nullptr);
h.addStmt(is);
EXPECT_EQ(repr(is), "store i64 10, i64* %loc1");
// error handling
- Bvariable *loc2 = be->local_variable(func, "loc2", bi64t, true, loc);
+ Bvariable *loc2 = be->local_variable(func, "loc2", bi64t, nullptr, true, loc);
Bstatement *bad = be->init_statement(func, loc2, be->error_expression());
ASSERT_TRUE(bad != nullptr);
EXPECT_EQ(bad, be->error_statement());
diff --git a/unittests/BackendCore/BackendTreeIntegrity.cpp b/unittests/BackendCore/BackendTreeIntegrity.cpp
index 77ab24f..a4c42e9 100644
--- a/unittests/BackendCore/BackendTreeIntegrity.cpp
+++ b/unittests/BackendCore/BackendTreeIntegrity.cpp
@@ -71,8 +71,8 @@
Location loc;
Bfunction *func = mkFunci32o64(be.get(), "foo");
Btype *bi64t = be->integer_type(false, 64);
- Bvariable *loc1 = be->local_variable(func, "loc1", bi64t, true, loc);
- Bvariable *loc2 = be->local_variable(func, "loc2", bi64t, true, loc);
+ Bvariable *loc1 = be->local_variable(func, "loc1", bi64t, nullptr, true, loc);
+ Bvariable *loc2 = be->local_variable(func, "loc2", bi64t, nullptr, true, loc);
// Create "loc1" varexpr, then supply to more than one statement
Bexpression *ve = be->var_expression(loc1, loc);
diff --git a/unittests/BackendCore/BackendVarTests.cpp b/unittests/BackendCore/BackendVarTests.cpp
index f94e7ed..fa4fe38 100644
--- a/unittests/BackendCore/BackendVarTests.cpp
+++ b/unittests/BackendCore/BackendVarTests.cpp
@@ -37,7 +37,7 @@
Bvariable *loc2 = h.mkLocal("loc2", bst);
ASSERT_TRUE(loc2 != nullptr);
EXPECT_TRUE(loc2 != be->error_variable());
- Bvariable *loc3 = be->local_variable(func2, "loc3", bst, false, loc);
+ Bvariable *loc3 = be->local_variable(func2, "loc3", bst, nullptr, false, loc);
ASSERT_TRUE(loc3 != nullptr);
EXPECT_TRUE(loc3 != be->error_variable());
@@ -59,7 +59,8 @@
EXPECT_EQ(repr(es), "%loc1.ld.0 = load i64, i64* %loc1");
// Make sure error detection is working
- Bvariable *loce = be->local_variable(func1, "", be->error_type(), true, loc);
+ Bvariable *loce = be->local_variable(func1, "", be->error_type(), nullptr,
+ true, loc);
EXPECT_TRUE(loce == be->error_variable());
bool broken = h.finish(PreserveDebugInfo);
@@ -502,8 +503,8 @@
Btype *bst = mkTwoFieldStruct(be, bi32t, bi32t);
// Block with two locals
- Bvariable *x = be->local_variable(func, "x", bi32t, false, loc);
- Bvariable *y = be->local_variable(func, "y", bst, false, loc);
+ Bvariable *x = be->local_variable(func, "x", bi32t, nullptr, false, loc);
+ Bvariable *y = be->local_variable(func, "y", bst, nullptr, false, loc);
const std::vector<Bvariable *> vars = { x, y };
Bblock *b1 = be->block(func, nullptr, vars, loc, loc);
Bstatement *is1 = be->init_statement(func, x, be->zero_expression(bi32t));
@@ -680,8 +681,52 @@
}
)RAW_RESULT";
bool isOK = h.expectValue(func->function(), exp);
- EXPECT_TRUE(isOK && "Value does not have expected contents");
+ EXPECT_TRUE(isOK && "Value does not have expected contents");
}
+TEST(BackendVarTests, MakeLocalWithDeclVar) {
+ FcnTestHarness h("foo");
+ Llvm_backend *be = h.be();
+ Bfunction *func1 = h.func();
+
+ // Create an initial local variable at the top level.
+ Location loc;
+ Btype *bi64t = be->integer_type(false, 64);
+ Bvariable *loc1 = h.mkLocal("loc1", bi64t);
+
+ // Now manufacture a second local that refers to the first via the
+ // "declVar" mechanism.
+ Bvariable *loc2 = be->local_variable(func1, "loc2", bi64t, loc1, false, loc);
+ Bstatement *is2 = be->init_statement(func1, loc2, mkInt64Const(be, 9));
+
+ // Third regular local.
+ Bvariable *loc3 = be->local_variable(func1, "loc3", bi64t, nullptr,
+ false, loc);
+ Bstatement *is3 = be->init_statement(func1, loc3, mkInt64Const(be, 11));
+
+ // Place the second and third variables in a block.
+ std::vector<Bvariable *> vars;
+ vars.push_back(loc2);
+ vars.push_back(loc3);
+ h.newBlock(&vars);
+
+ // Add inits to block
+ std::vector<Bstatement *> bstmts;
+ bstmts.push_back(is2);
+ bstmts.push_back(is3);
+ be->block_add_statements(h.block(), bstmts);
+
+ // The two vars should share the same alloca instruction.
+ EXPECT_TRUE(loc1->value() == loc2->value());
+
+ bool broken = h.finish(PreserveDebugInfo);
+ EXPECT_FALSE(broken && "Module failed to verify.");
+
+ // Expect to see only one lifetime start, since A) loc1 is at the top
+ // level, and B) loc2 uses loc1 as declVar.
+ const char *expected = "@llvm.lifetime.start.p0i8(i64";
+ EXPECT_EQ(h.countInstancesInModuleDump(expected), 1);
+}
+
}
diff --git a/unittests/BackendCore/TestUtils.cpp b/unittests/BackendCore/TestUtils.cpp
index 1ad3e79..716946b 100644
--- a/unittests/BackendCore/TestUtils.cpp
+++ b/unittests/BackendCore/TestUtils.cpp
@@ -95,6 +95,25 @@
return false;
}
+unsigned countinstances(const std::string &text, const std::string &pat)
+{
+ unsigned instances = 0;
+ std::vector<std::string> textToks = tokenize(text);
+ std::vector<std::string> patToks = tokenize(pat);
+ for (unsigned ti = 0; ti < textToks.size(); ++ti) {
+ bool fail = false;
+ for (unsigned tic = ti, pi = 0; pi < patToks.size(); ++pi, ++tic) {
+ if (tic >= textToks.size() || patToks[pi] != textToks[tic]) {
+ fail = true;
+ break;
+ }
+ }
+ if (!fail)
+ instances += 1;
+ }
+ return instances;
+}
+
std::string repr(llvm::Value *val) {
std::string res;
llvm::raw_string_ostream os(res);
@@ -520,7 +539,7 @@
Bexpression *init)
{
assert(func_);
- Bvariable *v = be()->local_variable(func_, name, typ, true, loc_);
+ Bvariable *v = be()->local_variable(func_, name, typ, nullptr, true, loc_);
if (!init)
init = be()->zero_expression(typ);
Bstatement *is = be()->init_statement(func_, v, init);
@@ -716,6 +735,15 @@
return containstokens(actual, expected);
}
+unsigned FcnTestHarness::countInstancesInModuleDump(const std::string &expected)
+{
+ std::string res;
+ llvm::raw_string_ostream os(res);
+ be()->module().print(os, nullptr);
+ std::string actual(trimsp(os.str()));
+ return countinstances(actual, expected);
+}
+
bool FcnTestHarness::expectRepr(Bnode *node, const std::string &expected)
{
std::string reason;
diff --git a/unittests/BackendCore/TestUtils.h b/unittests/BackendCore/TestUtils.h
index 0b4c203..986975f 100644
--- a/unittests/BackendCore/TestUtils.h
+++ b/unittests/BackendCore/TestUtils.h
@@ -235,6 +235,10 @@
// trouble here.
bool expectModuleDumpContains(const std::string &expected);
+ // Return a count of the number of times we see the specified
+ // token sequence within the module dump.
+ unsigned countInstancesInModuleDump(const std::string &expected);
+
// Skip the checking at finish time for orphan CFG blocks.
void allowOrphans() { findOrphanBBs_ = false; }
