From: Ian Lance Taylor Date: Thu, 13 Sep 2018 00:45:55 +0000 (+0000) Subject: compiler: omit a couple of write barriers X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=8a90f559d727a993b5a3c5c9c5d5cd5520a7503a;p=gcc.git compiler: omit a couple of write barriers Omit a write barrier for s = s[0:] for a slice s. In this case the pointer is not changing and no write barrier is required. Omit a write barrier for s = append(s, v) in the case where len(s) < cap(s) (and similarly when appending more values). When the slice has enough capacity the pointer is not changing and no write barrier is required. These changes are required to avoid write barriers in the method randomOrder.reset in the runtime package. That method is called from procresize, at a point where we do not want to allocate memory. Otherwise that method can use a write barrier, allocate memory, and break TestReadMemStats. Reviewed-on: https://go-review.googlesource.com/134219 From-SVN: r264259 --- diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE index 1ef2c8d39d3..1afbcb48b34 100644 --- a/gcc/go/gofrontend/MERGE +++ b/gcc/go/gofrontend/MERGE @@ -1,4 +1,4 @@ -06e688ff6d829c8de3735e9f59b61b373afc596f +acf852f838e6b99f907d84648be853fa2c374393 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/expressions.cc b/gcc/go/gofrontend/expressions.cc index efc8eba9908..a296777bd54 100644 --- a/gcc/go/gofrontend/expressions.cc +++ b/gcc/go/gofrontend/expressions.cc @@ -131,6 +131,40 @@ Expression::determine_type_no_context() this->do_determine_type(&context); } +// Return true if two expressions refer to the same variable or struct +// field. This can only be true when there are no side effects. + +bool +Expression::is_same_variable(Expression* a, Expression* b) +{ + if (a->classification() != b->classification()) + return false; + + Var_expression* av = a->var_expression(); + if (av != NULL) + return av->named_object() == b->var_expression()->named_object(); + + Field_reference_expression* af = a->field_reference_expression(); + if (af != NULL) + { + Field_reference_expression* bf = b->field_reference_expression(); + return (af->field_index() == bf->field_index() + && Expression::is_same_variable(af->expr(), bf->expr())); + } + + Unary_expression* au = a->unary_expression(); + if (au != NULL) + { + Unary_expression* bu = b->unary_expression(); + return (au->op() == OPERATOR_MULT + && bu->op() == OPERATOR_MULT + && Expression::is_same_variable(au->operand(), + bu->operand())); + } + + return false; +} + // Return an expression handling any conversions which must be done during // assignment. @@ -7421,7 +7455,7 @@ Builtin_call_expression::do_flatten(Gogo* gogo, Named_object* function, break; case BUILTIN_APPEND: - return this->flatten_append(gogo, function, inserter); + return this->flatten_append(gogo, function, inserter, NULL, NULL); case BUILTIN_COPY: { @@ -7658,11 +7692,18 @@ Builtin_call_expression::lower_make(Statement_inserter* inserter) // Flatten a call to the predeclared append function. We do this in // the flatten phase, not the lowering phase, so that we run after -// type checking and after order_evaluations. +// type checking and after order_evaluations. If ASSIGN_LHS is not +// NULL, this append is the right-hand-side of an assignment and +// ASSIGN_LHS is the left-hand-side; in that case, set LHS directly +// rather than returning a slice. This lets us omit a write barrier +// in common cases like a = append(a, ...) when the slice does not +// need to grow. ENCLOSING is not NULL iff ASSIGN_LHS is not NULL. Expression* Builtin_call_expression::flatten_append(Gogo* gogo, Named_object* function, - Statement_inserter* inserter) + Statement_inserter* inserter, + Expression* assign_lhs, + Block* enclosing) { if (this->is_error_expression()) return this; @@ -7679,6 +7720,8 @@ Builtin_call_expression::flatten_append(Gogo* gogo, Named_object* function, if (args->size() == 1) { // append(s) evaluates to s. + if (assign_lhs != NULL) + return NULL; return args->front(); } @@ -7795,14 +7838,46 @@ Builtin_call_expression::flatten_append(Gogo* gogo, Named_object* function, // FIXME: Mark this index as not requiring bounds checks. ref = Expression::make_index(ref, zero, ref2, NULL, loc); - Expression* rhs = Expression::make_conditional(cond, call, ref, loc); + if (assign_lhs == NULL) + { + Expression* rhs = Expression::make_conditional(cond, call, ref, loc); + + gogo->lower_expression(function, inserter, &rhs); + gogo->flatten_expression(function, inserter, &rhs); - gogo->lower_expression(function, inserter, &rhs); - gogo->flatten_expression(function, inserter, &rhs); + ref = Expression::make_temporary_reference(s1tmp, loc); + Statement* assign = Statement::make_assignment(ref, rhs, loc); + inserter->insert(assign); + } + else + { + gogo->lower_expression(function, inserter, &cond); + gogo->flatten_expression(function, inserter, &cond); + gogo->lower_expression(function, inserter, &call); + gogo->flatten_expression(function, inserter, &call); + gogo->lower_expression(function, inserter, &ref); + gogo->flatten_expression(function, inserter, &ref); - Expression* lhs = Expression::make_temporary_reference(s1tmp, loc); - Statement* assign = Statement::make_assignment(lhs, rhs, loc); - inserter->insert(assign); + Block* then_block = new Block(enclosing, loc); + Assignment_statement* assign = + Statement::make_assignment(assign_lhs, call, loc); + then_block->add_statement(assign); + + Block* else_block = new Block(enclosing, loc); + assign = Statement::make_assignment(assign_lhs->copy(), ref, loc); + // This assignment will not change the pointer value, so it does + // not need a write barrier. + assign->set_omit_write_barrier(); + else_block->add_statement(assign); + + Statement* s = Statement::make_if_statement(cond, then_block, + else_block, loc); + inserter->insert(s); + + ref = Expression::make_temporary_reference(s1tmp, loc); + assign = Statement::make_assignment(ref, assign_lhs->copy(), loc); + inserter->insert(assign); + } if (this->is_varargs()) { @@ -7839,12 +7914,17 @@ Builtin_call_expression::flatten_append(Gogo* gogo, Named_object* function, Expression* off = Expression::make_integer_ul(i, int_type, loc); ref2 = Expression::make_binary(OPERATOR_PLUS, ref2, off, loc); // FIXME: Mark this index as not requiring bounds checks. - lhs = Expression::make_index(ref, ref2, NULL, NULL, loc); + Expression* lhs = Expression::make_index(ref, ref2, NULL, NULL, + loc); gogo->lower_expression(function, inserter, &lhs); gogo->flatten_expression(function, inserter, &lhs); // The flatten pass runs after the write barrier pass, so we // need to insert a write barrier here if necessary. - if (!gogo->assign_needs_write_barrier(lhs)) + // However, if ASSIGN_LHS is not NULL, we have been called + // directly before the write barrier pass. + Statement* assign; + if (assign_lhs != NULL + || !gogo->assign_needs_write_barrier(lhs)) assign = Statement::make_assignment(lhs, *pa, loc); else { @@ -7856,6 +7936,9 @@ Builtin_call_expression::flatten_append(Gogo* gogo, Named_object* function, } } + if (assign_lhs != NULL) + return NULL; + return Expression::make_temporary_reference(s1tmp, loc); } diff --git a/gcc/go/gofrontend/expressions.h b/gcc/go/gofrontend/expressions.h index 5fa41715946..f53cc6e62aa 100644 --- a/gcc/go/gofrontend/expressions.h +++ b/gcc/go/gofrontend/expressions.h @@ -869,6 +869,11 @@ class Expression bool is_local_variable() const; + // Return true if two expressions refer to the same variable or + // struct field. + static bool + is_same_variable(Expression*, Expression*); + // Make the builtin function descriptor type, so that it can be // converted. static void @@ -2402,6 +2407,10 @@ class Builtin_call_expression : public Call_expression static bool array_len_is_constant(Expression* expr); + Expression* + flatten_append(Gogo*, Named_object*, Statement_inserter*, Expression*, + Block*); + protected: // This overrides Call_expression::do_lower. Expression* @@ -2459,8 +2468,6 @@ class Builtin_call_expression : public Call_expression Expression* lower_make(Statement_inserter*); - Expression* flatten_append(Gogo*, Named_object*, Statement_inserter*); - bool check_int_value(Expression*, bool is_length, bool* small); diff --git a/gcc/go/gofrontend/statements.cc b/gcc/go/gofrontend/statements.cc index c94d8cf369e..19a07d462ba 100644 --- a/gcc/go/gofrontend/statements.cc +++ b/gcc/go/gofrontend/statements.cc @@ -686,7 +686,7 @@ Assignment_statement::do_traverse_assignments(Traverse_assignments* tassign) } // Lower an assignment to a map index expression to a runtime function -// call. +// call. Mark some slice assignments as not requiring a write barrier. Statement* Assignment_statement::do_lower(Gogo*, Named_object*, Block* enclosing, @@ -750,6 +750,21 @@ Assignment_statement::do_lower(Gogo*, Named_object*, Block* enclosing, return Statement::make_block_statement(b, loc); } + // An assignment of the form s = s[:n] does not require a write + // barrier, because the pointer value will not change. + Array_index_expression* aie = this->rhs_->array_index_expression(); + if (aie != NULL + && aie->end() != NULL + && Expression::is_same_variable(this->lhs_, aie->array())) + { + Numeric_constant nc; + unsigned long ival; + if (aie->start()->numeric_constant_value(&nc) + && nc.to_unsigned_long(&ival) == Numeric_constant::NC_UL_VALID + && ival == 0) + this->omit_write_barrier_ = true; + } + return this; } @@ -876,7 +891,7 @@ Assignment_statement::do_dump_statement(Ast_dump_context* ast_dump_context) // Make an assignment statement. -Statement* +Assignment_statement* Statement::make_assignment(Expression* lhs, Expression* rhs, Location location) { diff --git a/gcc/go/gofrontend/statements.h b/gcc/go/gofrontend/statements.h index 852ab43ce2a..c9ab4d6fac3 100644 --- a/gcc/go/gofrontend/statements.h +++ b/gcc/go/gofrontend/statements.h @@ -148,7 +148,7 @@ class Statement make_temporary(Type*, Expression*, Location); // Make an assignment statement. - static Statement* + static Assignment_statement* make_assignment(Expression*, Expression*, Location); // Make an assignment operation (+=, etc.). @@ -562,7 +562,7 @@ class Assignment_statement : public Statement Assignment_statement(Expression* lhs, Expression* rhs, Location location) : Statement(STATEMENT_ASSIGNMENT, location), - lhs_(lhs), rhs_(rhs) + lhs_(lhs), rhs_(rhs), omit_write_barrier_(false) { } Expression* @@ -573,6 +573,14 @@ class Assignment_statement : public Statement rhs() const { return this->rhs_; } + bool + omit_write_barrier() const + { return this->omit_write_barrier_; } + + void + set_omit_write_barrier() + { this->omit_write_barrier_ = true; } + protected: int do_traverse(Traverse* traverse); @@ -603,6 +611,8 @@ class Assignment_statement : public Statement Expression* lhs_; // Right hand side--the rvalue. Expression* rhs_; + // True if we can omit a write barrier from this assignment. + bool omit_write_barrier_; }; // A statement which creates and initializes a temporary variable. diff --git a/gcc/go/gofrontend/wb.cc b/gcc/go/gofrontend/wb.cc index 99f467ef90d..58524cdb322 100644 --- a/gcc/go/gofrontend/wb.cc +++ b/gcc/go/gofrontend/wb.cc @@ -16,26 +16,87 @@ #include "runtime.h" #include "gogo.h" -// Mark variables whose addresses are taken. This has to be done -// before the write barrier pass and after the escape analysis pass. -// It would be nice to do this elsewhere but there isn't an obvious -// place. +// Mark variables whose addresses are taken and do some other +// cleanups. This has to be done before the write barrier pass and +// after the escape analysis pass. It would be nice to do this +// elsewhere but there isn't an obvious place. class Mark_address_taken : public Traverse { public: Mark_address_taken(Gogo* gogo) - : Traverse(traverse_expressions), - gogo_(gogo) + : Traverse(traverse_functions + | traverse_statements + | traverse_expressions), + gogo_(gogo), function_(NULL) { } + int + function(Named_object*); + + int + statement(Block*, size_t*, Statement*); + int expression(Expression**); private: + // General IR. Gogo* gogo_; + // The function we are traversing. + Named_object* function_; }; +// Record a function. + +int +Mark_address_taken::function(Named_object* no) +{ + go_assert(this->function_ == NULL); + this->function_ = no; + int t = no->func_value()->traverse(this); + this->function_ = NULL; + + if (t == TRAVERSE_EXIT) + return t; + return TRAVERSE_SKIP_COMPONENTS; +} + +// Traverse a statement. + +int +Mark_address_taken::statement(Block* block, size_t* pindex, Statement* s) +{ + // If this is an assignment of the form s = append(s, ...), expand + // it now, so that we can assign it to the left hand side in the + // middle of the expansion and possibly skip a write barrier. + Assignment_statement* as = s->assignment_statement(); + if (as != NULL && !as->lhs()->is_sink_expression()) + { + Call_expression* rce = as->rhs()->call_expression(); + if (rce != NULL + && rce->builtin_call_expression() != NULL + && (rce->builtin_call_expression()->code() + == Builtin_call_expression::BUILTIN_APPEND) + && Expression::is_same_variable(as->lhs(), rce->args()->front())) + { + Statement_inserter inserter = Statement_inserter(block, pindex); + Expression* a = + rce->builtin_call_expression()->flatten_append(this->gogo_, + this->function_, + &inserter, + as->lhs(), + block); + go_assert(a == NULL); + // That does the assignment, so remove this statement. + Expression* e = Expression::make_boolean(true, s->location()); + Statement* dummy = Statement::make_statement(e, true); + block->replace_statement(*pindex, dummy); + } + } + return TRAVERSE_CONTINUE; +} + // Mark variable addresses taken. int @@ -387,6 +448,10 @@ Write_barriers::statement(Block* block, size_t* pindex, Statement* s) case Statement::STATEMENT_ASSIGNMENT: { Assignment_statement* as = s->assignment_statement(); + + if (as->omit_write_barrier()) + break; + Expression* lhs = as->lhs(); Expression* rhs = as->rhs();