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
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;