From 1df5fce42a295ae8cb34c6a4aae30814679b0b59 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 13 Jun 2018 20:32:10 +0000 Subject: [PATCH] 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 Reviewed-on: https://go-review.googlesource.com/118637 From-SVN: r261568 --- gcc/go/gofrontend/MERGE | 2 +- gcc/go/gofrontend/gogo.cc | 3 +++ gcc/go/gofrontend/gogo.h | 17 ++++++++++++----- gcc/go/gofrontend/wb.cc | 17 +++++++++++++---- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE index f656fa3130e..6013430593c 100644 --- a/gcc/go/gofrontend/MERGE +++ b/gcc/go/gofrontend/MERGE @@ -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. diff --git a/gcc/go/gofrontend/gogo.cc b/gcc/go/gofrontend/gogo.cc index 4d77ace62e9..eb31aa20f51 100644 --- a/gcc/go/gofrontend/gogo.cc +++ b/gcc/go/gofrontend/gogo.cc @@ -8444,6 +8444,9 @@ Traverse::function_declaration(Named_object*) 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); diff --git a/gcc/go/gofrontend/gogo.h b/gcc/go/gofrontend/gogo.h index 139df1785d4..6511599f95a 100644 --- a/gcc/go/gofrontend/gogo.h +++ b/gcc/go/gofrontend/gogo.h @@ -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 diff --git a/gcc/go/gofrontend/wb.cc b/gcc/go/gofrontend/wb.cc index 094d8ec3534..99f467ef90d 100644 --- a/gcc/go/gofrontend/wb.cc +++ b/gcc/go/gofrontend/wb.cc @@ -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* no) 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* no) 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, size_t* pindex, Statement* s) 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, size_t* pindex, Statement* s) &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, size_t* pindex, Statement* s) // 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; -- 2.30.2