From e8e91b845496603168161015ccd5325524abbf7e Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Thu, 16 May 2019 04:35:15 +0000 Subject: [PATCH] compiler: improve escape analysis on interface conversions If an interface does not escape, it doesn't need a heap allocation to hold the data (for non-direct interface type). This CL improves the escape analysis to track interface conversions, and reduces these allocations. Implicit interface conversions were mostly added late in the compilation pipeline, after the escape analysis. For the escape analysis to see them, we move the introduction of these conversions earlier, right before the escape analysis. Now that the compiler can generate interface conversions inlined, gcc/testsuite/go.test/test/nilptr2.go needs to be adjusted as in golang.org/cl/176579, so the use function does an actual use. Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/176459 * go.test/test/nilptr2.go: Change use function to actually do something. From-SVN: r271276 --- gcc/go/gofrontend/MERGE | 2 +- gcc/go/gofrontend/escape.cc | 20 ++- gcc/go/gofrontend/expressions.cc | 179 ++++++++++++++++++++++++-- gcc/go/gofrontend/expressions.h | 40 +++++- gcc/go/gofrontend/go.cc | 4 + gcc/go/gofrontend/gogo.cc | 51 ++++++++ gcc/go/gofrontend/gogo.h | 8 ++ gcc/go/gofrontend/statements.cc | 56 ++++++++ gcc/go/gofrontend/statements.h | 22 ++++ gcc/testsuite/ChangeLog | 5 + gcc/testsuite/go.test/test/nilptr2.go | 5 +- 11 files changed, 369 insertions(+), 23 deletions(-) diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE index 672ad671bb3..fba3721f23d 100644 --- a/gcc/go/gofrontend/MERGE +++ b/gcc/go/gofrontend/MERGE @@ -1,4 +1,4 @@ -6112f9b8fa9d57d2db8a709cc8b44a94d778d08a +2df0879e7880057293c0a59be6868a3e6ea5105b 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/escape.cc b/gcc/go/gofrontend/escape.cc index e1c98094831..47c133150ec 100644 --- a/gcc/go/gofrontend/escape.cc +++ b/gcc/go/gofrontend/escape.cc @@ -2407,9 +2407,11 @@ Escape_analysis_assign::assign(Node* dst, Node* src) Type* tt = tce->type(); if ((ft->is_string_type() && tt->is_slice_type()) || (ft->is_slice_type() && tt->is_string_type()) - || (ft->integer_type() != NULL && tt->is_string_type())) + || (ft->integer_type() != NULL && tt->is_string_type()) + || tt->interface_type() != NULL) { - // string([]byte), string([]rune), []byte(string), []rune(string), string(rune) + // string([]byte), string([]rune), []byte(string), []rune(string), string(rune), + // interface(T) this->flows(dst, src); break; } @@ -3151,14 +3153,24 @@ Escape_analysis_flood::flood(Level level, Node* dst, Node* src, Type* tt = tce->type(); if ((ft->is_string_type() && tt->is_slice_type()) || (ft->is_slice_type() && tt->is_string_type()) - || (ft->integer_type() != NULL && tt->is_string_type())) + || (ft->integer_type() != NULL && tt->is_string_type()) + || tt->interface_type() != NULL) { - // string([]byte), string([]rune), []byte(string), []rune(string), string(rune) + // string([]byte), string([]rune), []byte(string), []rune(string), string(rune), + // interface(T) src->set_encoding(Node::ESCAPE_HEAP); if (debug_level != 0 && osrcesc != src->encoding()) go_inform(src->location(), "%s escapes to heap", src->ast_format(gogo).c_str()); extra_loop_depth = mod_loop_depth; + if (tt->interface_type() != NULL + && ft->has_pointer() + && !ft->is_direct_iface_type()) + // We're converting from a non-direct interface type. + // The interface will hold a heap copy of the data + // Flow the data to heap. See issue 29353. + this->flood(level, this->context_->sink(), + Node::make_node(tce->expr()), -1); } } else if (e->array_index_expression() != NULL diff --git a/gcc/go/gofrontend/expressions.cc b/gcc/go/gofrontend/expressions.cc index 54f6e525e1e..8af0dd43a88 100644 --- a/gcc/go/gofrontend/expressions.cc +++ b/gcc/go/gofrontend/expressions.cc @@ -184,11 +184,10 @@ Expression::convert_for_assignment(Gogo*, Type* lhs_type, NULL); if (!are_identical && lhs_type->interface_type() != NULL) { - if (rhs_type->interface_type() == NULL) - return Expression::convert_type_to_interface(lhs_type, rhs, location); - else - return Expression::convert_interface_to_interface(lhs_type, rhs, false, - location); + // Type to interface conversions have been made explicit early. + go_assert(rhs_type->interface_type() != NULL); + return Expression::convert_interface_to_interface(lhs_type, rhs, false, + location); } else if (!are_identical && rhs_type->interface_type() != NULL) return Expression::convert_interface_to_type(lhs_type, rhs, location); @@ -231,11 +230,12 @@ Expression::convert_for_assignment(Gogo*, Type* lhs_type, } // Return an expression for a conversion from a non-interface type to an -// interface type. +// interface type. If ON_STACK is true, it can allocate the storage on +// stack. Expression* Expression::convert_type_to_interface(Type* lhs_type, Expression* rhs, - Location location) + bool on_stack, Location location) { Interface_type* lhs_interface_type = lhs_type->interface_type(); bool lhs_is_empty = lhs_interface_type->is_empty(); @@ -302,9 +302,9 @@ Expression::convert_type_to_interface(Type* lhs_type, Expression* rhs, { // We are assigning a non-pointer value to the interface; the // interface gets a copy of the value in the heap if it escapes. - // TODO(cmang): Associate escape state state of RHS with newly - // created OBJ. obj = Expression::make_heap_expression(rhs, location); + if (on_stack) + obj->heap_expression()->set_allocate_on_stack(); } return Expression::make_interface_value(lhs_type, first_field, obj, location); @@ -3625,6 +3625,14 @@ Type_conversion_expression::do_flatten(Gogo*, Named_object*, inserter->insert(temp); this->expr_ = Expression::make_temporary_reference(temp, this->location()); } + + // For interface conversion, decide if we can allocate on stack. + if (this->type()->interface_type() != NULL) + { + Node* n = Node::make_node(this); + if ((n->encoding() & ESCAPE_MASK) == Node::ESCAPE_NONE) + this->no_escape_ = true; + } return this; } @@ -3812,12 +3820,20 @@ Type_conversion_expression::do_get_backend(Translate_context* context) Bexpression* bexpr = this->expr_->get_backend(context); return gogo->backend()->convert_expression(btype, bexpr, loc); } + else if (type->interface_type() != NULL + && expr_type->interface_type() == NULL) + { + Expression* conversion = + Expression::convert_type_to_interface(type, this->expr_, + this->no_escape_, loc); + return conversion->get_backend(context); + } else if (type->interface_type() != NULL || expr_type->interface_type() != NULL) { Expression* conversion = Expression::convert_for_assignment(gogo, type, this->expr_, - this->location()); + loc); return conversion->get_backend(context); } else if (type->is_string_type() @@ -8466,6 +8482,10 @@ Builtin_call_expression::flatten_append(Gogo* gogo, Named_object* function, lhs->array_index_expression()->set_needs_bounds_check(false); gogo->lower_expression(function, inserter, &lhs); gogo->flatten_expression(function, inserter, &lhs); + Expression* elem = *pa; + if (!Type::are_identical(element_type, elem->type(), 0, NULL) + && element_type->interface_type() != NULL) + elem = Expression::make_cast(element_type, elem, loc); // The flatten pass runs after the write barrier pass, so we // need to insert a write barrier here if necessary. // However, if ASSIGN_LHS is not NULL, we have been called @@ -8473,12 +8493,12 @@ Builtin_call_expression::flatten_append(Gogo* gogo, Named_object* function, Statement* assign; if (assign_lhs != NULL || !gogo->assign_needs_write_barrier(lhs)) - assign = Statement::make_assignment(lhs, *pa, loc); + assign = Statement::make_assignment(lhs, elem, loc); else { Function* f = function == NULL ? NULL : function->func_value(); assign = gogo->assign_with_write_barrier(f, NULL, inserter, - lhs, *pa, loc); + lhs, elem, loc); } inserter->insert(assign); } @@ -9840,7 +9860,7 @@ Builtin_call_expression::do_get_backend(Translate_context* context) Type::make_empty_interface_type(Linemap::predeclared_location()); Expression* nil = Expression::make_nil(location); - nil = Expression::convert_for_assignment(gogo, empty, nil, location); + nil = Expression::make_interface_value(empty, nil, nil, location); // We need to handle a deferred call to recover specially, // because it changes whether it can recover a panic or not. @@ -10474,6 +10494,46 @@ Call_expression::do_flatten(Gogo* gogo, Named_object*, return this; } +// Make implicit type conversions explicit. + +void +Call_expression::do_add_conversions() +{ + // Skip call that requires a thunk. We generate conversions inside the thunk. + if (this->is_concurrent_ || this->is_deferred_) + return; + + if (this->args_ == NULL || this->args_->empty()) + return; + + Function_type* fntype = this->get_function_type(); + if (fntype == NULL) + { + go_assert(saw_errors()); + return; + } + if (fntype->parameters() == NULL || fntype->parameters()->empty()) + return; + + Location loc = this->location(); + Expression_list::iterator pa = this->args_->begin(); + Typed_identifier_list::const_iterator pp = fntype->parameters()->begin(); + bool is_interface_method = + this->fn_->interface_field_reference_expression() != NULL; + if (!is_interface_method && fntype->is_method()) + { + // Skip the receiver argument, which cannot be interface. + pa++; + } + for (; pa != this->args_->end(); ++pa, ++pp) + { + Type* pt = pp->type(); + if (!Type::are_identical(pt, (*pa)->type(), 0, NULL) + && pt->interface_type() != NULL) + *pa = Expression::make_cast(pt, *pa, loc); + } +} + // Get the function type. This can return NULL in error cases. Function_type* @@ -12250,6 +12310,21 @@ Map_index_expression::do_check_types(Gogo*) } } +// Add explicit type conversions. + +void +Map_index_expression::do_add_conversions() +{ + Map_type* mt = this->get_map_type(); + if (mt == NULL) + return; + Type* lt = mt->key_type(); + Type* rt = this->index_->type(); + if (!Type::are_identical(lt, rt, 0, NULL) + && lt->interface_type() != NULL) + this->index_ = Expression::make_cast(lt, this->index_, this->location()); +} + // Get the backend representation for a map index. Bexpression* @@ -13450,6 +13525,33 @@ Struct_construction_expression::do_flatten(Gogo*, Named_object*, return this; } +// Make implicit type conversions explicit. + +void +Struct_construction_expression::do_add_conversions() +{ + if (this->vals() == NULL) + return; + + Location loc = this->location(); + const Struct_field_list* fields = this->type_->struct_type()->fields(); + Expression_list::iterator pv = this->vals()->begin(); + for (Struct_field_list::const_iterator pf = fields->begin(); + pf != fields->end(); + ++pf, ++pv) + { + if (pv == this->vals()->end()) + break; + if (*pv != NULL) + { + Type* ft = pf->type(); + if (!Type::are_identical(ft, (*pv)->type(), 0, NULL) + && ft->interface_type() != NULL) + *pv = Expression::make_cast(ft, *pv, loc); + } + } +} + // Return the backend representation for constructing a struct. Bexpression* @@ -13698,6 +13800,26 @@ Array_construction_expression::do_flatten(Gogo*, Named_object*, return this; } +// Make implicit type conversions explicit. + +void +Array_construction_expression::do_add_conversions() +{ + if (this->vals() == NULL) + return; + + Type* et = this->type_->array_type()->element_type(); + if (et->interface_type() == NULL) + return; + + Location loc = this->location(); + for (Expression_list::iterator pv = this->vals()->begin(); + pv != this->vals()->end(); + ++pv) + if (!Type::are_identical(et, (*pv)->type(), 0, NULL)) + *pv = Expression::make_cast(et, *pv, loc); +} + // Get a constructor expression for the array values. Bexpression* @@ -14221,6 +14343,37 @@ Map_construction_expression::do_copy() this->location()); } +// Make implicit type conversions explicit. + +void +Map_construction_expression::do_add_conversions() +{ + if (this->vals_ == NULL || this->vals_->empty()) + return; + + Map_type* mt = this->type_->map_type(); + Type* kt = mt->key_type(); + Type* vt = mt->val_type(); + bool key_is_interface = (kt->interface_type() != NULL); + bool val_is_interface = (vt->interface_type() != NULL); + if (!key_is_interface && !val_is_interface) + return; + + Location loc = this->location(); + for (Expression_list::iterator pv = this->vals_->begin(); + pv != this->vals_->end(); + ++pv) + { + if (key_is_interface && + !Type::are_identical(kt, (*pv)->type(), 0, NULL)) + *pv = Expression::make_cast(kt, *pv, loc); + ++pv; + if (val_is_interface && + !Type::are_identical(vt, (*pv)->type(), 0, NULL)) + *pv = Expression::make_cast(vt, *pv, loc); + } +} + // Return the backend representation for constructing a map. Bexpression* diff --git a/gcc/go/gofrontend/expressions.h b/gcc/go/gofrontend/expressions.h index 9ed81f11ea5..b1811ea2f1f 100644 --- a/gcc/go/gofrontend/expressions.h +++ b/gcc/go/gofrontend/expressions.h @@ -934,6 +934,11 @@ class Expression flatten(Gogo* gogo, Named_object* function, Statement_inserter* inserter) { return this->do_flatten(gogo, function, inserter); } + // Make implicit type conversions explicit. + void + add_conversions() + { this->do_add_conversions(); } + // Determine the real type of an expression with abstract integer, // floating point, or complex type. TYPE_CONTEXT describes the // expected type. @@ -1019,6 +1024,13 @@ class Expression Expression* rhs, bool for_type_guard, Location); + // Return an expression for a conversion from a non-interface type to an + // interface type. If ON_STACK is true, it can allocate the storage on + // stack. + static Expression* + convert_type_to_interface(Type* lhs_type, Expression* rhs, + bool on_stack, Location); + // Return a backend expression implementing the comparison LEFT OP RIGHT. // TYPE is the type of both sides. static Bexpression* @@ -1070,6 +1082,10 @@ class Expression do_flatten(Gogo*, Named_object*, Statement_inserter*) { return this; } + // Make implicit type conversions explicit. + virtual void + do_add_conversions() + { } // Return whether this is a constant expression. virtual bool @@ -1214,9 +1230,6 @@ class Expression : NULL); } - static Expression* - convert_type_to_interface(Type*, Expression*, Location); - static Expression* unpack_direct_iface(Expression*, Location); @@ -1674,7 +1687,7 @@ class Type_conversion_expression : public Expression Location location) : Expression(EXPRESSION_CONVERSION, location), type_(type), expr_(expr), may_convert_function_types_(false), - no_copy_(false) + no_copy_(false), no_escape_(false) { } // Return the type to which we are converting. @@ -1766,6 +1779,10 @@ class Type_conversion_expression : public Expression // True if a string([]byte) conversion can reuse the backing store // without copying. Only used in string([]byte) conversion. bool no_copy_; + // True if a conversion to interface does not escape, so it does + // not need a heap allocation. Only used in type-to-interface + // conversion. + bool no_escape_; }; // An unsafe type conversion, used to pass values to builtin functions. @@ -2402,6 +2419,9 @@ class Call_expression : public Expression void do_dump_expression(Ast_dump_context*) const; + void + do_add_conversions(); + private: bool check_argument_type(int, const Type*, const Type*, Location, bool); @@ -3162,6 +3182,9 @@ class Map_index_expression : public Expression void do_dump_expression(Ast_dump_context*) const; + void + do_add_conversions(); + private: // The map we are looking into. Expression* map_; @@ -3648,6 +3671,9 @@ class Struct_construction_expression : public Expression, void do_dump_expression(Ast_dump_context*) const; + void + do_add_conversions(); + private: // The type of the struct to construct. Type* type_; @@ -3721,6 +3747,9 @@ protected: virtual void dump_slice_storage_expression(Ast_dump_context*) const { } + void + do_add_conversions(); + private: // The type of the array to construct. Type* type_; @@ -3844,6 +3873,9 @@ class Map_construction_expression : public Expression void do_dump_expression(Ast_dump_context*) const; + void + do_add_conversions(); + private: // The type of the map to construct. Type* type_; diff --git a/gcc/go/gofrontend/go.cc b/gcc/go/gofrontend/go.cc index 183664a5662..5106e8d0434 100644 --- a/gcc/go/gofrontend/go.cc +++ b/gcc/go/gofrontend/go.cc @@ -142,6 +142,10 @@ go_parse_input_files(const char** filenames, unsigned int filename_count, if (only_check_syntax) return; + // Make implicit type conversions explicit. + ::gogo->add_conversions(); + + // Analyze the program flow for escape information. ::gogo->analyze_escape(); // Export global identifiers as appropriate. diff --git a/gcc/go/gofrontend/gogo.cc b/gcc/go/gofrontend/gogo.cc index 9f18e14aecd..06657cb2be4 100644 --- a/gcc/go/gofrontend/gogo.cc +++ b/gcc/go/gofrontend/gogo.cc @@ -2996,6 +2996,57 @@ Gogo::lower_constant(Named_object* no) lower.constant(no, false); } +// Make implicit type conversions explicit. Currently only does for +// interface conversions, so the escape analysis can see them and +// optimize. + +class Add_conversions : public Traverse +{ + public: + Add_conversions() + : Traverse(traverse_statements + | traverse_expressions) + { } + + int + statement(Block*, size_t* pindex, Statement*); + + int + expression(Expression**); +}; + +// Add explicit conversions in a statement. + +int +Add_conversions::statement(Block*, size_t*, Statement* sorig) +{ + sorig->add_conversions(); + return TRAVERSE_CONTINUE; +} + +// Add explicit conversions in an expression. + +int +Add_conversions::expression(Expression** pexpr) +{ + (*pexpr)->add_conversions(); + return TRAVERSE_CONTINUE; +} + +void +Gogo::add_conversions() +{ + Add_conversions add_conversions; + this->traverse(&add_conversions); +} + +void +Gogo::add_conversions_in_block(Block *b) +{ + Add_conversions add_conversions; + b->traverse(&add_conversions); +} + // Traverse the tree to create function descriptors as needed. class Create_function_descriptors : public Traverse diff --git a/gcc/go/gofrontend/gogo.h b/gcc/go/gofrontend/gogo.h index cfa238ac6e8..448da047316 100644 --- a/gcc/go/gofrontend/gogo.h +++ b/gcc/go/gofrontend/gogo.h @@ -683,6 +683,14 @@ class Gogo void check_return_statements(); + // Make implicit type conversions explicit. + void + add_conversions(); + + // Make implicit type conversions explicit in a block. + void + add_conversions_in_block(Block*); + // Analyze the program flow for escape information. void analyze_escape(); diff --git a/gcc/go/gofrontend/statements.cc b/gcc/go/gofrontend/statements.cc index cd1b60199e8..c850b49bebd 100644 --- a/gcc/go/gofrontend/statements.cc +++ b/gcc/go/gofrontend/statements.cc @@ -324,6 +324,22 @@ Variable_declaration_statement::do_flatten(Gogo* gogo, Named_object* function, return this; } +// Add explicit type conversions. + +void +Variable_declaration_statement::do_add_conversions() +{ + Variable* var = this->var_->var_value(); + Expression* init = var->init(); + if (init == NULL) + return; + Type* lt = var->type(); + Type* rt = init->type(); + if (!Type::are_identical(lt, rt, 0, NULL) + && lt->interface_type() != NULL) + var->set_init(Expression::make_cast(lt, init, this->location())); +} + // Convert a variable declaration to the backend representation. Bstatement* @@ -582,6 +598,20 @@ Temporary_statement::do_flatten(Gogo*, Named_object*, Block*, return this; } +// Add explicit type conversions. + +void +Temporary_statement::do_add_conversions() +{ + if (this->init_ == NULL) + return; + Type* lt = this->type(); + Type* rt = this->init_->type(); + if (!Type::are_identical(lt, rt, 0, NULL) + && lt->interface_type() != NULL) + this->init_ = Expression::make_cast(lt, this->init_, this->location()); +} + // Convert to backend representation. Bstatement* @@ -960,6 +990,18 @@ Assignment_statement::do_flatten(Gogo*, Named_object*, Block*, return this; } +// Add explicit type conversions. + +void +Assignment_statement::do_add_conversions() +{ + Type* lt = this->lhs_->type(); + Type* rt = this->rhs_->type(); + if (!Type::are_identical(lt, rt, 0, NULL) + && lt->interface_type() != NULL) + this->rhs_ = Expression::make_cast(lt, this->rhs_, this->location()); +} + // Convert an assignment statement to the backend representation. Bstatement* @@ -2638,6 +2680,8 @@ Thunk_statement::build_thunk(Gogo* gogo, const std::string& thunk_name) // just for the call statement now. The other types are known. call_statement->determine_types(); + gogo->add_conversions_in_block(b); + gogo->flatten_block(function, b); if (may_call_recover @@ -4543,6 +4587,18 @@ Send_statement::do_flatten(Gogo*, Named_object*, Block*, return this; } +// Add explicit type conversions. + +void +Send_statement::do_add_conversions() +{ + Type* lt = this->channel_->type()->channel_type()->element_type(); + Type* rt = this->val_->type(); + if (!Type::are_identical(lt, rt, 0, NULL) + && lt->interface_type() != NULL) + this->val_ = Expression::make_cast(lt, this->val_, this->location()); +} + // Convert a send statement to the backend representation. Bstatement* diff --git a/gcc/go/gofrontend/statements.h b/gcc/go/gofrontend/statements.h index 19ccb85a4bd..2985daa6a72 100644 --- a/gcc/go/gofrontend/statements.h +++ b/gcc/go/gofrontend/statements.h @@ -338,6 +338,11 @@ class Statement export_statement(Export_function_body* efb) { this->do_export_statement(efb); } + // Make implicit type conversions explicit. + void + add_conversions() + { this->do_add_conversions(); } + // Read a statement from export data. The location should be used // for the returned statement. Errors should be reported using the // Import_function_body's location method. @@ -534,6 +539,11 @@ class Statement virtual void do_dump_statement(Ast_dump_context*) const = 0; + // Implemented by child class: make implicit conversions explicit. + virtual void + do_add_conversions() + { } + // Traverse an expression in a statement. int traverse_expression(Traverse*, Expression**); @@ -645,6 +655,9 @@ class Assignment_statement : public Statement void do_dump_statement(Ast_dump_context*) const; + void + do_add_conversions(); + private: // Left hand side--the lvalue. Expression* lhs_; @@ -717,6 +730,9 @@ class Temporary_statement : public Statement void do_dump_statement(Ast_dump_context*) const; + void + do_add_conversions(); + private: // The type of the temporary variable. Type* type_; @@ -774,6 +790,9 @@ class Variable_declaration_statement : public Statement void do_dump_statement(Ast_dump_context*) const; + void + do_add_conversions(); + private: Named_object* var_; }; @@ -959,6 +978,9 @@ class Send_statement : public Statement void do_dump_statement(Ast_dump_context*) const; + void + do_add_conversions(); + private: // The channel on which to send the value. Expression* channel_; diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index a9922d67863..835963b8f03 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2019-05-15 Cherry Zhang + + * go.test/test/nilptr2.go: Change use function to actually do + something. + 2019-05-16 Jakub Jelinek PR middle-end/90478 diff --git a/gcc/testsuite/go.test/test/nilptr2.go b/gcc/testsuite/go.test/test/nilptr2.go index 57a5f8068f0..d2f4c912f64 100644 --- a/gcc/testsuite/go.test/test/nilptr2.go +++ b/gcc/testsuite/go.test/test/nilptr2.go @@ -35,7 +35,10 @@ var m *M var m1 *M1 var m2 *M2 -func use(interface{}) { +var V interface{} + +func use(x interface{}) { + V = x } var tests = []struct{ -- 2.30.2