Go patch committed: Avoid introducing redundant write barriers

Message ID CAOyqgcW7jFcorG+XKknAHMiRrZe4Q_BvQA6rjub13v8bt3P+2g@mail.gmail.com
State New
Headers show
Series
  • Go patch committed: Avoid introducing redundant write barriers
Related show

Commit Message

Ian Lance Taylor June 13, 2018, 8:32 p.m.
This patch to the Go frontend by Than McIntosh fixes the compier to
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.

This fixes https://golang.org/25867 .

Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian

Patch

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 261555)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@ 
-1f07926263b6d14edb6abd6a00e6385190d30d0e
+c3ef5bbf4e4271216b6f22621269d458599e8087
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/gogo.cc
===================================================================
--- gcc/go/gofrontend/gogo.cc	(revision 261521)
+++ gcc/go/gofrontend/gogo.cc	(working copy)
@@ -8444,6 +8444,9 @@  Traverse::function_declaration(Named_obj
 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);
Index: gcc/go/gofrontend/gogo.h
===================================================================
--- gcc/go/gofrontend/gogo.h	(revision 261521)
+++ gcc/go/gofrontend/gogo.h	(working copy)
@@ -3419,19 +3419,24 @@  class Traverse
 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 @@  class Statement_inserter
   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
Index: gcc/go/gofrontend/wb.cc
===================================================================
--- gcc/go/gofrontend/wb.cc	(revision 261521)
+++ gcc/go/gofrontend/wb.cc	(working copy)
@@ -213,7 +213,7 @@  class Write_barriers : public Traverse
  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 @@  class Write_barriers : public Traverse
   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 @@  Write_barriers::variable(Named_object* n
   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 @@  Write_barriers::variable(Named_object* n
 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 @@  Write_barriers::statement(Block* block,
 
 	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 @@  Write_barriers::statement(Block* block,
 								   &inserter,
 								   ref, init,
 								   loc);
+        this->statements_added_.insert(assign);
 
 	// Replace the old variable declaration statement with the new
 	// initialization.
@@ -391,12 +398,14 @@  Write_barriers::statement(Block* block,
 	// 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;