gollvm: delete dead expr's instructions before visiting children

Dead expression's instructions may use values defined in the
children. Uses need to be deleted before deleting definition.
Therefore deleting these instructions before visiting children.

Change-Id: Ieb66cec101889b88d4e04543638ae8ace3d4788d
Reviewed-on: https://go-review.googlesource.com/47931
Reviewed-by: Than McIntosh <thanm@google.com>
diff --git a/llvm-gofrontend/go-llvm.cpp b/llvm-gofrontend/go-llvm.cpp
index 0ff4173..1347e17 100644
--- a/llvm-gofrontend/go-llvm.cpp
+++ b/llvm-gofrontend/go-llvm.cpp
@@ -2335,17 +2335,37 @@
 llvm::BasicBlock *GenBlocks::walkExpr(llvm::BasicBlock *curblock,
                                       Bexpression *expr)
 {
+  // Delete dead instructions before visiting the children,
+  // as they may use values defined in the children. Uses
+  // need to be deleted before deleting definition.
+  if (!curblock) {
+    for (auto originst : expr->instructions())
+      originst->dropAllReferences();
+    for (auto originst : expr->instructions())
+      originst->deleteValue();
+    expr->clear();
+  }
+
   // Visit children first
   const std::vector<Bnode *> &kids = expr->children();
   for (auto &child : kids)
     curblock = walk(child, curblock);
 
-  // Now visit instructions for this expr
-  for (auto originst : expr->instructions()) {
-    if (!curblock) {
+  // In case it becomes dead after visiting some child...
+  if (!curblock) {
+    for (auto originst : expr->instructions())
+      originst->dropAllReferences();
+    for (auto originst : expr->instructions())
       originst->deleteValue();
-      continue;
-    }
+    expr->clear();
+  }
+
+  // Now visit instructions for this expr
+  // TODO: currently the control flow won't change from
+  // live to dead in this loop. Handle it, especially
+  // deallocate part of the instruction list, if it
+  // becomes necessary.
+  for (auto originst : expr->instructions()) {
     auto pair = postProcessInst(originst, curblock);
     auto inst = pair.first;
     if (dibuildhelper_)
diff --git a/unittests/BackendCore/BackendStmtTests.cpp b/unittests/BackendCore/BackendStmtTests.cpp
index 9c9b200..1e54203 100644
--- a/unittests/BackendCore/BackendStmtTests.cpp
+++ b/unittests/BackendCore/BackendStmtTests.cpp
@@ -122,6 +122,52 @@
   EXPECT_FALSE(broken && "Module failed to verify.");
 }
 
+TEST(BackendStmtTests, TestReturnStmt2) {
+  // Test that dead code after the return statement is handled
+  // correctly.
+
+  FcnTestHarness h("foo");
+  Llvm_backend *be = h.be();
+  Bfunction *func = h.func();
+  Location loc;
+
+  // var x int64 = 10
+  Btype *bi64t = be->integer_type(false, 64);
+  Bvariable *x = h.mkLocal("x", bi64t, mkInt64Const(be, 10));
+
+  // return x
+  Bexpression *ve1 = be->var_expression(x, VE_rvalue, loc);
+  h.mkReturn(ve1);
+
+  // some dead code
+  // return x+20
+  Bexpression *ve2 = be->var_expression(x, VE_rvalue, loc);
+  Bexpression *addexpr = be->binary_expression(OPERATOR_PLUS, ve2, mkInt64Const(be, 20), loc);
+  h.mkReturn(addexpr);
+
+  const char *exp = R"RAW_RESULT(
+    define i64 @foo(i8* nest %nest.0, i32 %param1, i32 %param2, i64* %param3) #0 {
+    entry:
+      %param1.addr = alloca i32
+      %param2.addr = alloca i32
+      %param3.addr = alloca i64*
+      %x = alloca i64
+      store i32 %param1, i32* %param1.addr
+      store i32 %param2, i32* %param2.addr
+      store i64* %param3, i64** %param3.addr
+      store i64 10, i64* %x
+      %x.ld.0 = load i64, i64* %x
+      ret i64 %x.ld.0
+    }
+  )RAW_RESULT";
+
+  bool broken = h.finish(StripDebugInfo);
+  EXPECT_FALSE(broken && "Module failed to verify.");
+
+  bool isOK = h.expectValue(func->function(), exp);
+  EXPECT_TRUE(isOK && "Function does not have expected contents");
+}
+
 TEST(BackendStmtTests, TestLabelGotoStmts) {
 
   FcnTestHarness h("foo");