commit c3ef5bbf4e4271216b6f22621269d458599e8087
author Than McIntosh <thanm@google.com> Thu Jun 07 14:57:08 2018 -0400
committer Ian Lance Taylor <iant@golang.org> Wed Jun 13 20:30:35 2018 +0000
tree 30f32ae18bebc0c0f95574898ebf1bf60f239fb7
parent 1f07926263b6d14edb6abd6a00e6385190d30d0e

compiler: avoid introducing redundant write barriers

The traversal used by the write barrier insertion phase can sometimes
wind up visiting new statements inserted during the traversal, which
then results in duplicate / redundant write barrier guards. Example
program to reproduce:

  package small
  type S struct {
	N *S
	K int
  }
  var G *S = &S{N: nil, K: 101}

This patch changes the traversal code to keep track of statements
already added and avoid processing them again later in the traversal.

Fixes golang/go#25867

Change-Id: Ie7a81b49e815aff7379d3d4ef7c7a864cebcff5b
Reviewed-on: https://go-review.googlesource.com/118637
Reviewed-by: Ian Lance Taylor <iant@golang.org>

go/gogo.cc

diff --git a/go/gogo.cc b/go/gogo.cc
index 4d77ace..eb31aa2 100644
--- a/go/gogo.cc
+++ b/go/gogo.cc
@@ -8444,6 +8444,9 @@
 void
 Statement_inserter::insert(Statement* s)
 {
+  if (this->statements_added_ != NULL)
+    this->statements_added_->insert(s);
+
   if (this->block_ != NULL)
     {
       go_assert(this->pindex_ != NULL);

go/gogo.h

diff --git a/go/gogo.h b/go/gogo.h
index 139df17..6511599 100644
--- a/go/gogo.h
+++ b/go/gogo.h
@@ -3419,19 +3419,24 @@
 class Statement_inserter
 {
  public:
+  typedef Unordered_set(Statement*) Statements;
+
   // Empty constructor.
   Statement_inserter()
-    : block_(NULL), pindex_(NULL), gogo_(NULL), var_(NULL)
+      : block_(NULL), pindex_(NULL), gogo_(NULL), var_(NULL),
+        statements_added_(NULL)
   { }
 
   // Constructor for a statement in a block.
-  Statement_inserter(Block* block, size_t *pindex)
-    : block_(block), pindex_(pindex), gogo_(NULL), var_(NULL)
+  Statement_inserter(Block* block, size_t *pindex, Statements *added = NULL)
+      : block_(block), pindex_(pindex), gogo_(NULL), var_(NULL),
+        statements_added_(added)
   { }
 
   // Constructor for a global variable.
-  Statement_inserter(Gogo* gogo, Variable* var)
-    : block_(NULL), pindex_(NULL), gogo_(gogo), var_(var)
+  Statement_inserter(Gogo* gogo, Variable* var, Statements *added = NULL)
+      : block_(NULL), pindex_(NULL), gogo_(gogo), var_(var),
+        statements_added_(added)
   { go_assert(var->is_global()); }
 
   // We use the default copy constructor and assignment operator.
@@ -3451,6 +3456,8 @@
   Gogo* gogo_;
   // The global variable, when looking at an initializer expression.
   Variable* var_;
+  // If non-null, a set to record new statements inserted (non-owned).
+  Statements* statements_added_;
 };
 
 // When translating the gogo IR into the backend data structure, this

go/wb.cc

diff --git a/go/wb.cc b/go/wb.cc
index 094d8ec..99f467e 100644
--- a/go/wb.cc
+++ b/go/wb.cc
@@ -213,7 +213,7 @@
  public:
   Write_barriers(Gogo* gogo)
     : Traverse(traverse_functions | traverse_variables | traverse_statements),
-      gogo_(gogo), function_(NULL)
+      gogo_(gogo), function_(NULL), statements_added_()
   { }
 
   int
@@ -230,6 +230,8 @@
   Gogo* gogo_;
   // Current function.
   Function* function_;
+  // Statements introduced.
+  Statement_inserter::Statements statements_added_;
 };
 
 // Traverse a function.  Just record it for later.
@@ -298,9 +300,10 @@
   Location loc = init->location();
   Expression* ref = Expression::make_var_reference(no, loc);
 
-  Statement_inserter inserter(this->gogo_, var);
+  Statement_inserter inserter(this->gogo_, var, &this->statements_added_);
   Statement* s = this->gogo_->assign_with_write_barrier(NULL, NULL, &inserter,
 							ref, init, loc);
+  this->statements_added_.insert(s);
 
   var->add_preinit_statement(this->gogo_, s);
   var->clear_init();
@@ -313,6 +316,9 @@
 int
 Write_barriers::statement(Block* block, size_t* pindex, Statement* s)
 {
+  if (this->statements_added_.find(s) != this->statements_added_.end())
+    return TRAVERSE_SKIP_COMPONENTS;
+
   switch (s->classification())
     {
     default:
@@ -355,7 +361,7 @@
 
 	Function* function = this->function_;
 	Location loc = init->location();
-	Statement_inserter inserter(block, pindex);
+	Statement_inserter inserter(block, pindex, &this->statements_added_);
 
 	// Insert the variable declaration statement with no
 	// initializer, so that the variable exists.
@@ -370,6 +376,7 @@
 								   &inserter,
 								   ref, init,
 								   loc);
+        this->statements_added_.insert(assign);
 
 	// Replace the old variable declaration statement with the new
 	// initialization.
@@ -391,12 +398,14 @@
 	// Change the assignment to use a write barrier.
 	Function* function = this->function_;
 	Location loc = as->location();
-	Statement_inserter inserter = Statement_inserter(block, pindex);
+	Statement_inserter inserter =
+            Statement_inserter(block, pindex, &this->statements_added_);
 	Statement* assign = this->gogo_->assign_with_write_barrier(function,
 								   block,
 								   &inserter,
 								   lhs, rhs,
 								   loc);
+        this->statements_added_.insert(assign);
 	block->replace_statement(*pindex, assign);
       }
       break;