From 729232db0b630ffb0a17dfcfa0850ff68ac35bbf Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 7 Oct 2016 04:14:35 +0000 Subject: [PATCH] compiler: stack allocate storage for temp slices. During the lowering phase, the variable arguments to a varargs call are packaged up into a temporary slice object; the storage for this slice was being unconditionally allocated on the heap. Heap allocation is not necessary, however, if the varargs call correspond to an "append", since the append runtime routine only reads the slice storage (as opposed to stashing away the storage pointer). Enhance the lowering code to keep the slice storage on the stack for append() calls, to improve performance. Addresses issue golang/go#17304. Reviewed-on: https://go-review.googlesource.com/30136 From-SVN: r240853 --- gcc/go/gofrontend/MERGE | 2 +- gcc/go/gofrontend/expressions.cc | 118 +++++++++++++++++++++++++------ gcc/go/gofrontend/expressions.h | 48 ++++++++++++- 3 files changed, 144 insertions(+), 24 deletions(-) diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE index a4dac9935a9..dc9e1f4942e 100644 --- a/gcc/go/gofrontend/MERGE +++ b/gcc/go/gofrontend/MERGE @@ -1,4 +1,4 @@ -60b84be3fa146d821dcd3939dad6336c89432cff +2431267d513804a3b1aa71adde9aefba9e3c3c59 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 64b0d3c8341..343d354b8a1 100644 --- a/gcc/go/gofrontend/expressions.cc +++ b/gcc/go/gofrontend/expressions.cc @@ -6977,7 +6977,7 @@ Builtin_call_expression::do_lower(Gogo* gogo, Named_object* function, Type* element_type = slice_type->array_type()->element_type(); this->lower_varargs(gogo, function, inserter, Type::make_array_type(element_type, NULL), - 2); + 2, SLICE_STORAGE_DOES_NOT_ESCAPE); } break; @@ -8853,7 +8853,7 @@ Call_expression::do_lower(Gogo* gogo, Named_object* function, go_assert(parameters != NULL && !parameters->empty()); Type* varargs_type = parameters->back().type(); this->lower_varargs(gogo, function, inserter, varargs_type, - parameters->size()); + parameters->size(), SLICE_STORAGE_MAY_ESCAPE); } // If this is call to a method, call the method directly passing the @@ -8958,7 +8958,8 @@ Call_expression::do_lower(Gogo* gogo, Named_object* function, void Call_expression::lower_varargs(Gogo* gogo, Named_object* function, Statement_inserter* inserter, - Type* varargs_type, size_t param_count) + Type* varargs_type, size_t param_count, + Slice_storage_escape_disp escape_disp) { if (this->varargs_are_lowered_) return; @@ -9027,8 +9028,11 @@ Call_expression::lower_varargs(Gogo* gogo, Named_object* function, continue; vals->push_back(*pa); } - Expression* val = + Slice_construction_expression* sce = Expression::make_slice_composite_literal(varargs_type, vals, loc); + if (escape_disp == SLICE_STORAGE_DOES_NOT_ESCAPE) + sce->set_storage_does_not_escape(); + Expression* val = sce; gogo->lower_expression(function, inserter, &val); new_args->push_back(val); } @@ -12280,7 +12284,7 @@ Array_construction_expression::do_export(Export* exp) const exp->write_c_string(")"); } -// Dump ast representation of an array construction expressin. +// Dump ast representation of an array construction expression. void Array_construction_expression::do_dump_expression( @@ -12295,6 +12299,7 @@ Array_construction_expression::do_dump_expression( } ast_dump_context->ostream() << "]" ; ast_dump_context->dump_type(this->type_); + this->dump_slice_storage_expression(ast_dump_context); ast_dump_context->ostream() << "{" ; if (this->indexes_ == NULL) ast_dump_context->dump_expression_list(this->vals_); @@ -12350,7 +12355,8 @@ Slice_construction_expression::Slice_construction_expression( Expression_list* vals, Location location) : Array_construction_expression(EXPRESSION_SLICE_CONSTRUCTION, type, indexes, vals, location), - valtype_(NULL) + valtype_(NULL), array_val_(NULL), slice_storage_(NULL), + storage_escapes_(true) { go_assert(type->is_slice_type()); @@ -12371,7 +12377,6 @@ Slice_construction_expression::Slice_construction_expression( this->valtype_ = Type::make_array_type(element_type, length); } - // Traversal. int @@ -12382,23 +12387,29 @@ Slice_construction_expression::do_traverse(Traverse* traverse) return TRAVERSE_EXIT; if (Type::traverse(this->valtype_, traverse) == TRAVERSE_EXIT) return TRAVERSE_EXIT; + if (this->array_val_ != NULL + && Expression::traverse(&this->array_val_, traverse) == TRAVERSE_EXIT) + return TRAVERSE_EXIT; + if (this->slice_storage_ != NULL + && Expression::traverse(&this->slice_storage_, traverse) == TRAVERSE_EXIT) + return TRAVERSE_EXIT; return TRAVERSE_CONTINUE; } -// Return the backend representation for constructing a slice. +// Helper routine to create fixed array value underlying the slice literal. +// May be called during flattening, or later during do_get_backend(). -Bexpression* -Slice_construction_expression::do_get_backend(Translate_context* context) +Expression* +Slice_construction_expression::create_array_val() { Array_type* array_type = this->type()->array_type(); if (array_type == NULL) { go_assert(this->type()->is_error()); - return context->backend()->error_expression(); + return NULL; } Location loc = this->location(); - Type* element_type = array_type->element_type(); go_assert(this->valtype_ != NULL); Expression_list* vals = this->vals(); @@ -12408,11 +12419,71 @@ Slice_construction_expression::do_get_backend(Translate_context* context) vals = new Expression_list; vals->push_back(NULL); } - Expression* array_val = - new Fixed_array_construction_expression(this->valtype_, this->indexes(), - vals, loc); + return new Fixed_array_construction_expression( + this->valtype_, this->indexes(), vals, loc); +} + +// If we're previous established that the slice storage does not +// escape, then create a separate array temp val here for it. We +// need to do this as part of flattening so as to be able to insert +// the new temp statement. + +Expression* +Slice_construction_expression::do_flatten(Gogo* gogo, Named_object* no, + Statement_inserter* inserter) +{ + if (this->type()->array_type() == NULL) + return NULL; + + // Base class flattening first + this->Array_construction_expression::do_flatten(gogo, no, inserter); + + // Create an stack-allocated storage temp if storage won't escape + if (!this->storage_escapes_) + { + Location loc = this->location(); + this->array_val_ = create_array_val(); + go_assert(this->array_val_); + Temporary_statement* temp = + Statement::make_temporary(this->valtype_, this->array_val_, loc); + inserter->insert(temp); + this->slice_storage_ = Expression::make_temporary_reference(temp, loc); + } + return this; +} + +// When dumping a slice construction expression that has an explicit +// storeage temp, emit the temp here (if we don't do this the storage +// temp appears unused in the AST dump). + +void +Slice_construction_expression:: +dump_slice_storage_expression(Ast_dump_context* ast_dump_context) const +{ + if (this->slice_storage_ == NULL) + return; + ast_dump_context->ostream() << "storage=" ; + ast_dump_context->dump_expression(this->slice_storage_); +} + +// Return the backend representation for constructing a slice. + +Bexpression* +Slice_construction_expression::do_get_backend(Translate_context* context) +{ + if (this->array_val_ == NULL) + this->array_val_ = create_array_val(); + if (this->array_val_ == NULL) + { + go_assert(this->type()->is_error()); + return context->backend()->error_expression(); + } + + Location loc = this->location(); + Array_type* array_type = this->type()->array_type(); + Type* element_type = array_type->element_type(); - bool is_constant_initializer = array_val->is_immutable(); + bool is_constant_initializer = this->array_val_->is_immutable(); // We have to copy the initial values into heap memory if we are in // a function or if the values are not constants. We also have to @@ -12424,15 +12495,21 @@ Slice_construction_expression::do_get_backend(Translate_context* context) && !context->is_const())); Expression* space; - if (!copy_to_heap) + + if (this->slice_storage_ != NULL) + { + go_assert(!this->storage_escapes_); + space = Expression::make_unary(OPERATOR_AND, this->slice_storage_, loc); + } + else if (!copy_to_heap) { // The initializer will only run once. - space = Expression::make_unary(OPERATOR_AND, array_val, loc); + space = Expression::make_unary(OPERATOR_AND, this->array_val_, loc); space->unary_expression()->set_is_slice_init(); } else { - space = Expression::make_heap_expression(array_val, loc); + space = Expression::make_heap_expression(this->array_val_, loc); Node* n = Node::make_node(this); if ((n->encoding() & ESCAPE_MASK) == int(Node::ESCAPE_NONE)) { @@ -12442,7 +12519,6 @@ Slice_construction_expression::do_get_backend(Translate_context* context) } // Build a constructor for the slice. - Expression* len = this->valtype_->array_type()->length(); Expression* slice_val = Expression::make_slice_value(this->type(), space, len, len, loc); @@ -12452,7 +12528,7 @@ Slice_construction_expression::do_get_backend(Translate_context* context) // Make a slice composite literal. This is used by the type // descriptor code. -Expression* +Slice_construction_expression* Expression::make_slice_composite_literal(Type* type, Expression_list* vals, Location location) { diff --git a/gcc/go/gofrontend/expressions.h b/gcc/go/gofrontend/expressions.h index 357f7dbf9be..6db6eddc529 100644 --- a/gcc/go/gofrontend/expressions.h +++ b/gcc/go/gofrontend/expressions.h @@ -374,7 +374,7 @@ class Expression make_array_composite_literal(Type*, Expression_list*, Location); // Make a slice composite literal. - static Expression* + static Slice_construction_expression* make_slice_composite_literal(Type*, Expression_list*, Location); // Take an expression and allocate it on the heap. @@ -1067,6 +1067,18 @@ class Expression virtual void do_dump_expression(Ast_dump_context*) const = 0; + // Varargs lowering creates a slice object (unnamed compiler temp) + // to contain the variable length collection of values. The enum + // below tells the lowering routine whether it can mark that temp + // as non-escaping or not. For general varargs calls it is not always + // safe to stack-allocated the storage, but for specific cases (ex: + // call to append()) it is legal. + enum Slice_storage_escape_disp + { + SLICE_STORAGE_MAY_ESCAPE, + SLICE_STORAGE_DOES_NOT_ESCAPE + }; + private: // Convert to the desired statement classification, or return NULL. // This is a controlled dynamic cast. @@ -2128,7 +2140,8 @@ class Call_expression : public Expression // Let a builtin expression lower varargs. void lower_varargs(Gogo*, Named_object* function, Statement_inserter* inserter, - Type* varargs_type, size_t param_count); + Type* varargs_type, size_t param_count, + Slice_storage_escape_disp escape_disp); // Let a builtin expression check whether types have been // determined. @@ -3282,6 +3295,9 @@ protected: void do_dump_expression(Ast_dump_context*) const; + virtual void + dump_slice_storage_expression(Ast_dump_context*) const { } + private: // The type of the array to construct. Type* type_; @@ -3326,6 +3342,18 @@ class Slice_construction_expression : public Array_construction_expression Slice_construction_expression(Type* type, const std::vector* indexes, Expression_list* vals, Location location); + + Expression* + do_flatten(Gogo*, Named_object*, Statement_inserter*); + + // Record that the storage for this slice (e.g. vals) cannot escape, + // hence it can be stack-allocated. + void + set_storage_does_not_escape() + { + this->storage_escapes_ = false; + } + protected: // Note that taking the address of a slice literal is invalid. @@ -3345,9 +3373,25 @@ class Slice_construction_expression : public Array_construction_expression Bexpression* do_get_backend(Translate_context*); + void + dump_slice_storage_expression(Ast_dump_context* ast_dump_context) const; + + // Create an array value for the constructed slice. Invoked during + // flattening if slice storage does not escape, otherwise invoked + // later on during do_get_backend(). + Expression* + create_array_val(); + private: // The type of the values in this slice. Type* valtype_; + // Array value expression, optionally filled in during flattening. + Expression* array_val_; + // Slice storage expression, optionally filled in during flattening. + Expression* slice_storage_; + // Normally true. Can be set to false if we know that the resulting + // storage for the slice cannot escape. + bool storage_escapes_; }; // Construct a map. -- 2.30.2