gollvm: cleanup of Bnode tree deletion/removal utilities
Move the Bnode::destroy method into BnodeBuilder, and incorporate
support for deleting arbitrary Bnode trees (with a mix of Bstatements
and Bexpressions) instead of just expression trees. Other small
cleanups.
Change-Id: I7b4f623a6543f62720d3f734df5f8fdd28a2159d
Reviewed-on: https://go-review.googlesource.com/47910
Reviewed-by: Cherry Zhang <cherryyz@google.com>
diff --git a/llvm-gofrontend/go-llvm-bnode.cpp b/llvm-gofrontend/go-llvm-bnode.cpp
index 82ff9d8..c49bc16 100644
--- a/llvm-gofrontend/go-llvm-bnode.cpp
+++ b/llvm-gofrontend/go-llvm-bnode.cpp
@@ -1,4 +1,4 @@
-//===-- bnode.cpp - implementation of 'Bnode' class ---=======================//
+//===-- go-llvm-bnode.cpp - implementation of 'Bnode' class ---------------===//
//
// The LLVM Compiler Infrastructure
//
@@ -267,8 +267,19 @@
kid->osdump(os, ilevel + 2, linemap, terse);
}
-void Bnode::destroy(Bnode *node, WhichDel which)
+void BnodeBuilder::destroy(Bnode *node, WhichDel which)
{
+ std::set<Bnode *> visited;
+ destroyRec(node, which, visited);
+}
+
+void BnodeBuilder::destroyRec(Bnode *node,
+ WhichDel which,
+ std::set<Bnode *> &visited)
+{
+ if (visited.find(node) != visited.end())
+ return;
+ visited.insert(node);
if (which != DelWrappers) {
Bexpression *expr = node->castToBexpression();
if (expr) {
@@ -279,9 +290,9 @@
}
}
for (auto &kid : node->kids_)
- destroy(kid, which);
+ destroyRec(kid, which, visited);
if (which != DelInstructions)
- delete node;
+ freeNode(node);
}
SwitchDescriptor *Bnode::getSwitchCases()
@@ -291,12 +302,6 @@
return u.swcases;
}
-// only for unit testing, not for general use.
-void Bnode::removeAllChildren()
-{
- kids_.clear();
-}
-
Blabel *Bnode::label() const
{
assert(flavor() == N_LabelStmt || flavor() == N_GotoStmt ||
@@ -369,13 +374,22 @@
swcases_.clear();
}
-void BnodeBuilder::freeExpr(Bexpression *expr)
+void BnodeBuilder::freeNode(Bnode *node)
{
- assert(expr);
- earchive_[expr->id()] = nullptr;
- if (expr->id() == earchive_.size()-1)
- earchive_.pop_back();
- delete expr;
+ assert(node);
+ Bexpression *expr = node->castToBexpression();
+ if (expr) {
+ earchive_[expr->id()] = nullptr;
+ if (expr->id() == earchive_.size()-1)
+ earchive_.pop_back();
+ delete expr;
+ } else {
+ Bstatement *stmt = node->castToBstatement();
+ sarchive_[stmt->id()] = nullptr;
+ if (stmt->id() == sarchive_.size()-1)
+ sarchive_.pop_back();
+ delete stmt;
+ }
}
void BnodeBuilder::checkTreeInteg(Bnode *node)
@@ -672,9 +686,9 @@
std::vector<Bnode *> kids = { st, expr };
Bexpression *rval =
new Bexpression(N_Compound, kids, expr->value(), expr->btype(), loc);
+ rval->setTag(expr->tag());
if (expr->varExprPending())
rval->setVarExprPending(expr->varContext());
- rval->setTag(expr->tag());
return archive(rval);
}
@@ -937,7 +951,7 @@
integrityVisitor_->deletePending(node);
Bexpression *expr = node->castToBexpression();
assert(expr); // statements not yet supported, could be if needed
- freeExpr(expr);
+ freeNode(expr);
return orphans;
}
diff --git a/llvm-gofrontend/go-llvm-bnode.h b/llvm-gofrontend/go-llvm-bnode.h
index 833d7e2..9c8ce2e 100644
--- a/llvm-gofrontend/go-llvm-bnode.h
+++ b/llvm-gofrontend/go-llvm-bnode.h
@@ -149,12 +149,6 @@
void osdump(llvm::raw_ostream &os, unsigned ilevel = 0,
Llvm_linemap *linemap = nullptr, bool terse = false);
- // Delete some or all or this Bnode and its component
- // pieces. Deallocates just the Bnode, its contained instructions,
- // or both (depending on setting of 'which'). This is used mainly in
- // unit testing.
- static void destroy(Bnode *node, WhichDel which = DelWrappers);
-
// Cast to Bexpression. Returns NULL if not correct flavor.
Bexpression *castToBexpression() const;
@@ -198,10 +192,6 @@
Bnode(const Bnode &src);
SwitchDescriptor *getSwitchCases();
- // mainly for unit testing, not for general use.
- void removeAllChildren();
-
-
private:
void replaceChild(unsigned idx, Bnode *newchild);
@@ -347,8 +337,15 @@
// var is not an unadopted temp, then NULL is returned.
Bvariable *adoptTemporaryVariable(llvm::AllocaInst *alloca);
- // Free up this expr (it is garbage). Does not free up children.
- void freeExpr(Bexpression *expr);
+ // Free up this expr or stmt (it is garbage). Does not free up children.
+ void freeNode(Bnode *node);
+
+ // Delete some or all or this Bnode and its children/contents.
+ // Here 'which' can be set to DelInstructions (deletes only the
+ // LLVM instructions found in the subtree), DelWrappers (deletes
+ // Bnodes but not instructions), or DelBoth (gets rid of nodes and
+ // instructions).
+ void destroy(Bnode *node, WhichDel which = DelWrappers);
// Clone an expression subtree.
Bexpression *cloneSubtree(Bexpression *expr);
@@ -381,6 +378,7 @@
Bexpression *cloneSub(Bexpression *expr,
std::map<llvm::Value *, llvm::Value *> &vm);
void checkTreeInteg(Bnode *node);
+ void destroyRec(Bnode *node, WhichDel which, std::set<Bnode *> &visited);
private:
std::unique_ptr<Bstatement> errorStatement_;
diff --git a/llvm-gofrontend/go-llvm-bstatement.cpp b/llvm-gofrontend/go-llvm-bstatement.cpp
index dc23b25..64180ba 100644
--- a/llvm-gofrontend/go-llvm-bstatement.cpp
+++ b/llvm-gofrontend/go-llvm-bstatement.cpp
@@ -187,8 +187,3 @@
{
vars_.push_back(var);
}
-
-void Bblock::clearStatements()
-{
- removeAllChildren();
-}
diff --git a/llvm-gofrontend/go-llvm-bstatement.h b/llvm-gofrontend/go-llvm-bstatement.h
index ecdac15..536ba0a 100644
--- a/llvm-gofrontend/go-llvm-bstatement.h
+++ b/llvm-gofrontend/go-llvm-bstatement.h
@@ -159,9 +159,6 @@
// the backend temporary_variable() method-- allow for this here.
void addTemporaryVariable(Bvariable *var);
- // Exposed for unit testing the tree integrity checker. Not for general use.
- void clearStatements();
-
private:
friend class BnodeBuilder;
Bblock(Bfunction *func,
diff --git a/llvm-gofrontend/go-llvm.cpp b/llvm-gofrontend/go-llvm.cpp
index dac46a3..0ff4173 100644
--- a/llvm-gofrontend/go-llvm.cpp
+++ b/llvm-gofrontend/go-llvm.cpp
@@ -450,7 +450,7 @@
valbtype vbt(std::make_pair(val, btype));
auto it = valueExprmap_.find(vbt);
if (it != valueExprmap_.end()) {
- nbuilder_.freeExpr(expr);
+ nbuilder_.freeNode(expr);
return it->second;
}
valueExprmap_[vbt] = expr;
@@ -2494,7 +2494,7 @@
// Steal this return.
cachedReturn_ = rst;
} else {
- Bnode::destroy(re, DelInstructions);
+ be_->nodeBuilder().destroy(re, DelInstructions);
}
llvm::BranchInst::Create(finallyBlock_, curblock);
} else {
diff --git a/unittests/BackendCore/BackendTreeIntegrity.cpp b/unittests/BackendCore/BackendTreeIntegrity.cpp
index 000d630..c0fa2dc 100644
--- a/unittests/BackendCore/BackendTreeIntegrity.cpp
+++ b/unittests/BackendCore/BackendTreeIntegrity.cpp
@@ -72,9 +72,10 @@
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);
// Create "loc1" varexpr, then supply to more than one statement
- Bexpression *ve = be->var_expression(loc1, VE_lvalue, loc);
+ Bexpression *ve = be->var_expression(loc1, VE_rvalue, loc);
Bstatement *es1 = be->expression_statement(func, ve);
Bblock *block = mkBlockFromStmt(be.get(), func, es1);
Bstatement *es2 = be->expression_statement(func, ve);
@@ -86,7 +87,9 @@
EXPECT_FALSE(result.first);
EXPECT_TRUE(containstokens(result.second, "expr has multiple parents"));
- Bexpression *ve3= be->var_expression(loc1, VE_lvalue, loc);
+ be->nodeBuilder().destroy(block, DelBoth);
+
+ Bexpression *ve3 = be->var_expression(loc2, VE_rvalue, loc);
Bstatement *es3 = be->expression_statement(func, ve3);
Bblock *block2 = mkBlockFromStmt(be.get(), func, es3);
