gollvm: introduce nil check size threshold
Support for configuring size threshold for nil checks introduced
by the front end (requires corresponding gofrontend change
https://go-review.googlesource.com/c/gofrontend/+/80996).
Also in this patch is a small set of tweaks to the tree integrity
checker (better unit tests, support for unsharing conditional exprs).
Change-Id: Id8aef7db97c7b463ac4512b9551c2d96e2f7fe24
Reviewed-on: https://go-review.googlesource.com/81475
Reviewed-by: Cherry Zhang <cherryyz@google.com>
diff --git a/goparse-llvm.cpp b/goparse-llvm.cpp
index 0fcf275..0d8548b 100644
--- a/goparse-llvm.cpp
+++ b/goparse-llvm.cpp
@@ -383,6 +383,7 @@
args.check_divide_overflow = CheckDivideOverflow;
args.compiling_runtime = CompilingRuntime;
args.debug_escape_level = EscapeDebugLevel;
+ args.nil_check_size_threshold = -1;
args.linemap = linemap_.get();
args.backend = bridge_.get();
go_create_gogo (&args);
diff --git a/llvm-gofrontend/go-c.h b/llvm-gofrontend/go-c.h
index 213efe0..74a3f58 100644
--- a/llvm-gofrontend/go-c.h
+++ b/llvm-gofrontend/go-c.h
@@ -47,6 +47,7 @@
bool check_divide_overflow;
bool compiling_runtime;
int debug_escape_level;
+ int64_t nil_check_size_threshold;
};
extern void go_create_gogo (const struct go_create_gogo_args*);
diff --git a/llvm-gofrontend/go-llvm-bnode.cpp b/llvm-gofrontend/go-llvm-bnode.cpp
index 6190aa0..d0e92b0 100644
--- a/llvm-gofrontend/go-llvm-bnode.cpp
+++ b/llvm-gofrontend/go-llvm-bnode.cpp
@@ -354,6 +354,14 @@
return u.func;
}
+Bfunction *Bnode::getCallTarget() const
+{
+ assert(flavor() == N_Call);
+ if (kids_[0]->flavor() == N_FcnAddress)
+ return kids_[0]->getFunction();
+ return nullptr;
+}
+
int Bnode::getIndices() const
{
assert(flavor() == N_Composite);
diff --git a/llvm-gofrontend/go-llvm-bnode.h b/llvm-gofrontend/go-llvm-bnode.h
index 20d753c..f841bda 100644
--- a/llvm-gofrontend/go-llvm-bnode.h
+++ b/llvm-gofrontend/go-llvm-bnode.h
@@ -168,10 +168,14 @@
// Return struct field index for a field expr
unsigned fieldIndex() const;
- // Return Bfunction operand (valid only for function constants, calls,
- // and conditionals).
+ // Return function associated with this node. Value only for
+ // function constants, calls, and conditionals. Note that for calls and
+ // conditionals this will be the function containing the construct.
Bfunction *getFunction() const;
+ // Return target function for call (will be nil if indirect call).
+ Bfunction *getCallTarget() const;
+
// Return index to BnodeBuilder's vector of indices for composite,
// or -1 if no indexing.
int getIndices() const;
diff --git a/llvm-gofrontend/go-llvm-tree-integrity.cpp b/llvm-gofrontend/go-llvm-tree-integrity.cpp
index 405ab49..2dbc70f 100644
--- a/llvm-gofrontend/go-llvm-tree-integrity.cpp
+++ b/llvm-gofrontend/go-llvm-tree-integrity.cpp
@@ -18,6 +18,26 @@
#include "llvm/IR/Instruction.h"
+IntegrityVisitor::IntegrityVisitor(Llvm_backend *be,
+ TreeIntegCtl control)
+ : be_(be), ss_(str_), control_(control),
+ instShareCount_(0), stmtShareCount_(0), exprShareCount_(0)
+{
+ acceptableNodes_.insert(N_Const);
+ acceptableNodes_.insert(N_Call); // subject to acceptable target
+ acceptableNodes_.insert(N_FcnAddress);
+ acceptableNodes_.insert(N_Conditional);
+ acceptableNodes_.insert(N_Var);
+ acceptableNodes_.insert(N_Conversion);
+ acceptableNodes_.insert(N_Deref);
+ acceptableNodes_.insert(N_StructField);
+ acceptableNodes_.insert(N_BinaryOp); // subject to acceptableOpcodes
+ acceptableOpcodes_.insert(OPERATOR_PLUS);
+ acceptableOpcodes_.insert(OPERATOR_MINUS);
+ acceptableOpcodes_.insert(OPERATOR_EQEQ);
+ acceptableOpcodes_.insert(OPERATOR_NOTEQ);
+}
+
void IntegrityVisitor::dumpTag(const char *tag, void *ptr) {
ss_ << tag << ": ";
if (includePointers() == DumpPointers)
@@ -147,12 +167,13 @@
}
// capture a description of the error
- ss_ << "error: " << wh << " has multiple parents\n";
+ ss_ << "error: " << wh << " has multiple parents"
+ << " [ID=" << child->id() << "]\n";
ss_ << "child " << wh << ":\n";
dump(child);
- ss_ << "parent 1:\n";
+ ss_ << "parent 1: [ID=" << prevParent->id() << "]\n";
dump(prevParent);
- ss_ << "parent 2:\n";
+ ss_ << "parent 2: [ID=" << parent->id() << "]\n";
dump(parent);
return;
}
@@ -184,16 +205,6 @@
bool IntegrityVisitor::repairableSubTree(Bexpression *root)
{
- std::set<NodeFlavor> acceptable;
- acceptable.insert(N_Const);
- acceptable.insert(N_Var);
- acceptable.insert(N_Conversion);
- acceptable.insert(N_Deref);
- acceptable.insert(N_StructField);
-
- // Allow a limited set of these.
- acceptable.insert(N_BinaryOp);
-
std::set<Bexpression *> visited;
visited.insert(root);
std::vector<Bexpression *> workList;
@@ -202,11 +213,16 @@
while (! workList.empty()) {
Bexpression *e = workList.back();
workList.pop_back();
- if (acceptable.find(e->flavor()) == acceptable.end())
+ if (acceptableNodes_.find(e->flavor()) == acceptableNodes_.end())
return false;
+ if (e->flavor() == N_Call) {
+ // ok to duplicate runtime error calls, but not other calls
+ Bfunction *target = e->getCallTarget();
+ if (!target || target->name() != "__go_runtime_error")
+ return false;
+ }
if (e->flavor() == N_BinaryOp &&
- (e->op() != OPERATOR_PLUS &&
- e->op() != OPERATOR_MINUS))
+ acceptableOpcodes_.find(e->op()) == acceptableOpcodes_.end())
return false;
for (auto &c : e->children()) {
Bexpression *ce = c->castToBexpression();
diff --git a/llvm-gofrontend/go-llvm-tree-integrity.h b/llvm-gofrontend/go-llvm-tree-integrity.h
index aa7347c..4bf3e8e 100644
--- a/llvm-gofrontend/go-llvm-tree-integrity.h
+++ b/llvm-gofrontend/go-llvm-tree-integrity.h
@@ -71,10 +71,7 @@
class IntegrityVisitor {
public:
- IntegrityVisitor(Llvm_backend *be,
- TreeIntegCtl control)
- : be_(be), ss_(str_), control_(control),
- instShareCount_(0), stmtShareCount_(0), exprShareCount_(0) { }
+ IntegrityVisitor(Llvm_backend *be, TreeIntegCtl control);
// Visits the node tree "n", looking for any shared nodes or
// instructions. Returns TRUE if there is no sharing (or if all
@@ -103,12 +100,17 @@
// node, so please remove any outstanding references to it.
void deletePending(Bnode *node);
+ // Exposed for unit testing.
+ bool repairableSubTree(Bexpression *root);
+
private:
Llvm_backend *be_;
typedef std::pair<Bnode *, unsigned> parslot; // parent and child index
std::unordered_map<llvm::Instruction *, parslot> iparent_;
std::unordered_map<Bnode *, parslot> nparent_;
pairhashset<Bnode *, unsigned> sharing_;
+ std::set<NodeFlavor> acceptableNodes_;
+ std::set<Operator> acceptableOpcodes_;
std::string str_;
llvm::raw_string_ostream ss_;
TreeIntegCtl control_;
@@ -119,7 +121,6 @@
private:
bool repair(Bnode *n);
void visit(Bnode *n);
- bool repairableSubTree(Bexpression *root);
bool shouldBeTracked(Bnode *child);
void setParent(Bnode *child, Bnode *parent, unsigned slot);
void setParent(llvm::Instruction *inst, Bexpression *par, unsigned slot);
diff --git a/unittests/BackendCore/BackendTreeIntegrity.cpp b/unittests/BackendCore/BackendTreeIntegrity.cpp
index a03dc2b..78f0a2e 100644
--- a/unittests/BackendCore/BackendTreeIntegrity.cpp
+++ b/unittests/BackendCore/BackendTreeIntegrity.cpp
@@ -128,4 +128,78 @@
be->function_set_body(func, block2);
}
+TEST(BackendTreeIntegrity, CheckTreeIntegrityRepairableSubtree) {
+ FcnTestHarness h;
+ Llvm_backend *be = h.be();
+ Btype *bi32t = be->integer_type(false, 32);
+ Btype *bpi32t = be->pointer_type(bi32t);
+ BFunctionType *befty = mkFuncTyp(be, L_PARM, bpi32t, L_END);
+ Bfunction *func = h.mkFunction("x", befty);
+ Location loc;
+
+ TreeIntegCtl ctl(NoDumpPointers, ReportRepairableSharing, BatchMode);
+ IntegrityVisitor ivis(be, ctl);
+
+ // *p0 + 2
+ Bvariable *p0v = func->getNthParamVar(0);
+ Bexpression *vex0 = be->var_expression(p0v, loc);
+ Bexpression *deref = be->indirect_expression(bi32t, vex0, false, loc);
+ Bexpression *add =
+ be->binary_expression(OPERATOR_PLUS, mkInt32Const(be, 2), deref, loc);
+ EXPECT_TRUE(ivis.repairableSubTree(vex0));
+ EXPECT_TRUE(ivis.repairableSubTree(deref));
+ EXPECT_TRUE(ivis.repairableSubTree(add));
+
+ // p0 == nil ? 1 : *p0
+ Bexpression *vex2 = be->var_expression(p0v, loc);
+ Bexpression *npe = be->nil_pointer_expression();
+ Bexpression *cmp =
+ be->binary_expression(OPERATOR_EQEQ, npe, vex2, loc);
+ Bexpression *der2 = be->indirect_expression(bi32t, vex2, false, loc);
+ Bexpression *const1 = mkInt32Const(be, 1);
+ Bexpression *condex = be->conditional_expression(func, bi32t, cmp, const1,
+ der2, loc);
+ EXPECT_TRUE(ivis.repairableSubTree(cmp));
+ EXPECT_TRUE(ivis.repairableSubTree(condex));
+
+ // Not legal to share calls (in general)
+ Bexpression *vex3 = be->var_expression(p0v, loc);
+ Bexpression *call2 = h.mkCallExpr(be, func, vex3, nullptr);
+ EXPECT_FALSE(ivis.repairableSubTree(call2));
+
+ // Create runtime error function.
+ const char *rtename = "__go_runtime_error";
+ BFunctionType *bfterr = mkFuncTyp(be,
+ L_PARM, bi32t,
+ L_END);
+ bool visible = true; bool is_inl = true;
+ bool split_stack = false; bool uniq_sec = false;
+ bool is_decl = true;
+ Bfunction *rtefcn = be->function(bfterr, rtename, rtename, visible, is_decl,
+ is_inl, split_stack, uniq_sec, loc);
+
+ // p0 != nil ? *p0 + 3 : runtime_error(6)
+ Bexpression *cmp2 =
+ be->binary_expression(OPERATOR_NOTEQ, vex3, npe, loc);
+ Bexpression *der3 = be->indirect_expression(bi32t, vex3, false, loc);
+ Bexpression *add2 =
+ be->binary_expression(OPERATOR_PLUS, mkInt32Const(be, 3), der3, loc);
+ Bexpression *const6 = mkInt32Const(be, 6);
+ Bexpression *call3 = h.mkCallExpr(be, rtefcn, const6, nullptr);
+ Bexpression *condex2 = be->conditional_expression(func, bi32t, cmp2, add2,
+ call3, loc);
+
+ // Runtime error calls are ok to unshare/duplicate.
+ EXPECT_TRUE(ivis.repairableSubTree(call3));
+ EXPECT_TRUE(ivis.repairableSubTree(condex2));
+
+ h.mkExprStmt(call2);
+ h.mkExprStmt(condex);
+ h.mkExprStmt(condex2);
+ h.mkExprStmt(add);
+
+ bool broken = h.finish(StripDebugInfo);
+ EXPECT_FALSE(broken && "Module failed to verify.");
+}
+
}
