This patch to the Go frontend omits a couple of write barriers.
We 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.
We 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.
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 264245)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -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.
Index: gcc/go/gofrontend/expressions.cc
===================================================================
--- gcc/go/gofrontend/expressions.cc (revision 264245)
+++ gcc/go/gofrontend/expressions.cc (working copy)
@@ -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
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(Stat
// 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(
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(
// 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(
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(
}
}
+ if (assign_lhs != NULL)
+ return NULL;
+
return Expression::make_temporary_reference(s1tmp, loc);
}
Index: gcc/go/gofrontend/expressions.h
===================================================================
--- gcc/go/gofrontend/expressions.h (revision 264245)
+++ gcc/go/gofrontend/expressions.h (working copy)
@@ -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 C
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 C
Expression*
lower_make(Statement_inserter*);
- Expression* flatten_append(Gogo*, Named_object*, Statement_inserter*);
-
bool
check_int_value(Expression*, bool is_length, bool* small);
Index: gcc/go/gofrontend/statements.cc
===================================================================
--- gcc/go/gofrontend/statements.cc (revision 264245)
+++ gcc/go/gofrontend/statements.cc (working copy)
@@ -686,7 +686,7 @@ Assignment_statement::do_traverse_assign
}
// 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*, Na
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(
// Make an assignment statement.
-Statement*
+Assignment_statement*
Statement::make_assignment(Expression* lhs, Expression* rhs,
Location location)
{
Index: gcc/go/gofrontend/statements.h
===================================================================
--- gcc/go/gofrontend/statements.h (revision 264245)
+++ gcc/go/gofrontend/statements.h (working copy)
@@ -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 Stat
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 Stat
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 Stat
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.
Index: gcc/go/gofrontend/wb.cc
===================================================================
--- gcc/go/gofrontend/wb.cc (revision 264245)
+++ gcc/go/gofrontend/wb.cc (working copy)
@@ -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,
case Statement::STATEMENT_ASSIGNMENT:
{
Assignment_statement* as = s->assignment_statement();
+
+ if (as->omit_write_barrier())
+ break;
+
Expression* lhs = as->lhs();
Expression* rhs = as->rhs();
