From fdcef314bcb0728d883cd87d6950d85217e82755 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Mon, 7 Jan 2019 21:44:06 +0000 Subject: [PATCH] compiler: move slice construction to callers of makeslice This is the gccgo version of https://golang.org/cl/141822: Only return a pointer p to the new slices backing array from makeslice. Makeslice callers then construct sliceheader{p, len, cap} explictly instead of makeslice returning the slice. This change caused the GCC backend to break the runtime/pprof test by merging together the identical functions allocateReflectTransient and allocateTransient2M. This caused the traceback to be other than expected. Fix that by making the functions not identical. This is a step toward updating libgo to the Go1.12beta1 release. Reviewed-on: https://go-review.googlesource.com/c/155937 From-SVN: r267660 --- gcc/go/gofrontend/MERGE | 2 +- gcc/go/gofrontend/escape.cc | 12 +++ gcc/go/gofrontend/expressions.cc | 117 +++++++++++---------------- gcc/go/gofrontend/expressions.h | 57 +++++++++++++ gcc/go/gofrontend/runtime.def | 4 +- gcc/go/gofrontend/wb.cc | 55 +++++++++---- libgo/go/runtime/pprof/mprof_test.go | 4 +- libgo/go/runtime/slice.go | 7 +- 8 files changed, 166 insertions(+), 92 deletions(-) diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE index d6fe5f6afb9..7c7370a5954 100644 --- a/gcc/go/gofrontend/MERGE +++ b/gcc/go/gofrontend/MERGE @@ -1,4 +1,4 @@ -c257303eaef143663216e483857d5b259e05753f +c8a9bccbc524381d150c84907a61ac257c1b07cc 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 b56b4d0b57d..e1c98094831 100644 --- a/gcc/go/gofrontend/escape.cc +++ b/gcc/go/gofrontend/escape.cc @@ -1737,6 +1737,16 @@ Escape_analysis_assign::expression(Expression** pexpr) } break; + case Expression::EXPRESSION_SLICE_VALUE: + { + // Connect the pointer field to the slice value. + Node* slice_node = Node::make_node(*pexpr); + Node* ptr_node = + Node::make_node((*pexpr)->slice_value_expression()->valmem()); + this->assign(slice_node, ptr_node); + } + break; + case Expression::EXPRESSION_HEAP: { Node* pointer_node = Node::make_node(*pexpr); @@ -2263,6 +2273,8 @@ Escape_analysis_assign::assign(Node* dst, Node* src) // DST = map[T]V{...}. case Expression::EXPRESSION_STRUCT_CONSTRUCTION: // DST = T{...}. + case Expression::EXPRESSION_SLICE_VALUE: + // DST = slice{ptr, len, cap} case Expression::EXPRESSION_ALLOCATION: // DST = new(T). case Expression::EXPRESSION_BOUND_METHOD: diff --git a/gcc/go/gofrontend/expressions.cc b/gcc/go/gofrontend/expressions.cc index ef8a917c290..71f18002733 100644 --- a/gcc/go/gofrontend/expressions.cc +++ b/gcc/go/gofrontend/expressions.cc @@ -7787,21 +7787,29 @@ Builtin_call_expression::lower_make(Statement_inserter* inserter) Expression* call; if (is_slice) { + Temporary_statement* len_temp = NULL; + if (!len_arg->is_constant()) + { + len_temp = Statement::make_temporary(NULL, len_arg, loc); + inserter->insert(len_temp); + len_arg = Expression::make_temporary_reference(len_temp, loc); + } + if (cap_arg == NULL) { cap_small = len_small; - if (len_arg->numeric_constant_value(&nclen) - && nclen.to_unsigned_long(&vlen) == Numeric_constant::NC_UL_VALID) - cap_arg = Expression::make_integer_ul(vlen, len_arg->type(), loc); - else - { - Temporary_statement* temp = Statement::make_temporary(NULL, - len_arg, - loc); - inserter->insert(temp); - len_arg = Expression::make_temporary_reference(temp, loc); - cap_arg = Expression::make_temporary_reference(temp, loc); - } + if (len_temp == NULL) + cap_arg = len_arg->copy(); + else + cap_arg = Expression::make_temporary_reference(len_temp, loc); + } + else if (!cap_arg->is_constant()) + { + Temporary_statement* cap_temp = Statement::make_temporary(NULL, + cap_arg, + loc); + inserter->insert(cap_temp); + cap_arg = Expression::make_temporary_reference(cap_temp, loc); } Type* et = type->array_type()->element_type(); @@ -7809,7 +7817,12 @@ Builtin_call_expression::lower_make(Statement_inserter* inserter) Runtime::Function code = Runtime::MAKESLICE; if (!len_small || !cap_small) code = Runtime::MAKESLICE64; - call = Runtime::make_call(code, loc, 3, type_arg, len_arg, cap_arg); + Expression* mem = Runtime::make_call(code, loc, 3, type_arg, len_arg, + cap_arg); + mem = Expression::make_unsafe_cast(Type::make_pointer_type(et), mem, + loc); + call = Expression::make_slice_value(type, mem, len_arg->copy(), + cap_arg->copy(), loc); } else if (is_map) { @@ -13585,9 +13598,13 @@ Slice_construction_expression::do_get_backend(Translate_context* context) go_assert(this->storage_escapes_ || this->element_count() == 0); space = Expression::make_heap_expression(this->array_val_, loc); } + Array_type* at = this->valtype_->array_type(); + Type* et = at->element_type(); + space = Expression::make_unsafe_cast(Type::make_pointer_type(et), + space, loc); // Build a constructor for the slice. - Expression* len = this->valtype_->array_type()->length(); + Expression* len = at->length(); Expression* slice_val = Expression::make_slice_value(this->type(), space, len, len, loc); return slice_val->get_backend(context); @@ -15354,72 +15371,33 @@ Expression::make_slice_info(Expression* slice, Slice_info slice_info, return new Slice_info_expression(slice, slice_info, location); } -// An expression that represents a slice value: a struct with value pointer, -// length, and capacity fields. - -class Slice_value_expression : public Expression -{ - public: - Slice_value_expression(Type* type, Expression* valptr, Expression* len, - Expression* cap, Location location) - : Expression(EXPRESSION_SLICE_VALUE, location), - type_(type), valptr_(valptr), len_(len), cap_(cap) - { } - - protected: - int - do_traverse(Traverse*); - - Type* - do_type() - { return this->type_; } - - void - do_determine_type(const Type_context*) - { go_unreachable(); } - - Expression* - do_copy() - { - return new Slice_value_expression(this->type_->copy_expressions(), - this->valptr_->copy(), - this->len_->copy(), this->cap_->copy(), - this->location()); - } - - Bexpression* - do_get_backend(Translate_context* context); - - void - do_dump_expression(Ast_dump_context*) const; - - private: - // The type of the slice value. - Type* type_; - // The pointer to the values in the slice. - Expression* valptr_; - // The length of the slice. - Expression* len_; - // The capacity of the slice. - Expression* cap_; -}; +// Class Slice_value_expression. int Slice_value_expression::do_traverse(Traverse* traverse) { if (Type::traverse(this->type_, traverse) == TRAVERSE_EXIT - || Expression::traverse(&this->valptr_, traverse) == TRAVERSE_EXIT + || Expression::traverse(&this->valmem_, traverse) == TRAVERSE_EXIT || Expression::traverse(&this->len_, traverse) == TRAVERSE_EXIT || Expression::traverse(&this->cap_, traverse) == TRAVERSE_EXIT) return TRAVERSE_EXIT; return TRAVERSE_CONTINUE; } +Expression* +Slice_value_expression::do_copy() +{ + return new Slice_value_expression(this->type_->copy_expressions(), + this->valmem_->copy(), + this->len_->copy(), this->cap_->copy(), + this->location()); +} + Bexpression* Slice_value_expression::do_get_backend(Translate_context* context) { std::vector vals(3); - vals[0] = this->valptr_->get_backend(context); + vals[0] = this->valmem_->get_backend(context); vals[1] = this->len_->get_backend(context); vals[2] = this->cap_->get_backend(context); @@ -15434,7 +15412,7 @@ Slice_value_expression::do_dump_expression( { ast_dump_context->ostream() << "slicevalue("; ast_dump_context->ostream() << "values: "; - this->valptr_->dump_expression(ast_dump_context); + this->valmem_->dump_expression(ast_dump_context); ast_dump_context->ostream() << ", length: "; this->len_->dump_expression(ast_dump_context); ast_dump_context->ostream() << ", capacity: "; @@ -15443,11 +15421,14 @@ Slice_value_expression::do_dump_expression( } Expression* -Expression::make_slice_value(Type* at, Expression* valptr, Expression* len, +Expression::make_slice_value(Type* at, Expression* valmem, Expression* len, Expression* cap, Location location) { go_assert(at->is_slice_type()); - return new Slice_value_expression(at, valptr, len, cap, location); + go_assert(valmem->is_nil_expression() + || (at->array_type()->element_type() + == valmem->type()->points_to())); + return new Slice_value_expression(at, valmem, len, cap, location); } // An expression that evaluates to some characteristic of a non-empty interface. diff --git a/gcc/go/gofrontend/expressions.h b/gcc/go/gofrontend/expressions.h index a18322cfb91..db40d8d0d21 100644 --- a/gcc/go/gofrontend/expressions.h +++ b/gcc/go/gofrontend/expressions.h @@ -61,6 +61,7 @@ class Map_construction_expression; class Type_guard_expression; class Heap_expression; class Receive_expression; +class Slice_value_expression; class Conditional_expression; class Compound_expression; class Numeric_constant; @@ -841,6 +842,12 @@ class Expression receive_expression() { return this->convert(); } + // If this is a slice value expression, return the Slice_valiue_expression + // structure. Otherwise, return NULL. + Slice_value_expression* + slice_value_expression() + { return this->convert(); } + // If this is a conditional expression, return the Conditional_expression // structure. Otherwise, return NULL. Conditional_expression* @@ -3955,6 +3962,56 @@ class Receive_expression : public Expression Temporary_statement* temp_receiver_; }; +// An expression that represents a slice value: a struct with value pointer, +// length, and capacity fields. + +class Slice_value_expression : public Expression +{ + public: + Slice_value_expression(Type* type, Expression* valmem, Expression* len, + Expression* cap, Location location) + : Expression(EXPRESSION_SLICE_VALUE, location), + type_(type), valmem_(valmem), len_(len), cap_(cap) + { } + + // The memory holding the values in the slice. The type should be a + // pointer to the element value of the slice. + Expression* + valmem() const + { return this->valmem_; } + + protected: + int + do_traverse(Traverse*); + + Type* + do_type() + { return this->type_; } + + void + do_determine_type(const Type_context*) + { } + + Expression* + do_copy(); + + Bexpression* + do_get_backend(Translate_context* context); + + void + do_dump_expression(Ast_dump_context*) const; + + private: + // The type of the slice value. + Type* type_; + // The memory holding the values in the slice. + Expression* valmem_; + // The length of the slice. + Expression* len_; + // The capacity of the slice. + Expression* cap_; +}; + // Conditional expressions. class Conditional_expression : public Expression diff --git a/gcc/go/gofrontend/runtime.def b/gcc/go/gofrontend/runtime.def index ed759e80780..ded62514832 100644 --- a/gcc/go/gofrontend/runtime.def +++ b/gcc/go/gofrontend/runtime.def @@ -85,10 +85,10 @@ DEF_GO_RUNTIME(COMPLEX128_DIV, "__go_complex128_div", // Make a slice. DEF_GO_RUNTIME(MAKESLICE, "runtime.makeslice", P3(TYPE, INT, INT), - R1(SLICE)) + R1(POINTER)) DEF_GO_RUNTIME(MAKESLICE64, "runtime.makeslice64", P3(TYPE, INT64, INT64), - R1(SLICE)) + R1(POINTER)) // Make a map with a hint and an (optional, unused) map structure. diff --git a/gcc/go/gofrontend/wb.cc b/gcc/go/gofrontend/wb.cc index ccd318d0440..8620e405b2e 100644 --- a/gcc/go/gofrontend/wb.cc +++ b/gcc/go/gofrontend/wb.cc @@ -41,6 +41,9 @@ class Mark_address_taken : public Traverse expression(Expression**); private: + Call_expression* + find_makeslice_call(Expression*); + // General IR. Gogo* gogo_; // The function we are traversing. @@ -97,6 +100,31 @@ Mark_address_taken::statement(Block* block, size_t* pindex, Statement* s) return TRAVERSE_CONTINUE; } +// Look through the expression of a Slice_value_expression's valmem to +// find an call to makeslice. + +Call_expression* +Mark_address_taken::find_makeslice_call(Expression* expr) +{ + Unsafe_type_conversion_expression* utce = + expr->unsafe_conversion_expression(); + if (utce != NULL) + expr = utce->expr(); + + Call_expression* call = expr->call_expression(); + if (call == NULL) + return NULL; + + Func_expression* fe = call->fn()->func_expression(); + if (fe != NULL && fe->runtime_code() == Runtime::MAKESLICE) + return call; + + // We don't worry about MAKESLICE64 bcause we don't want to use a + // stack allocation for a large slice anyhow. + + return NULL; +} + // Mark variable addresses taken. int @@ -142,16 +170,12 @@ Mark_address_taken::expression(Expression** pexpr) } // Rewrite non-escaping makeslice with constant size to stack allocation. - Unsafe_type_conversion_expression* uce = - expr->unsafe_conversion_expression(); - if (uce != NULL - && uce->type()->is_slice_type() - && Node::make_node(uce->expr())->encoding() == Node::ESCAPE_NONE - && uce->expr()->call_expression() != NULL) + Slice_value_expression* sve = expr->slice_value_expression(); + if (sve != NULL) { - Call_expression* call = uce->expr()->call_expression(); - if (call->fn()->func_expression() != NULL - && call->fn()->func_expression()->runtime_code() == Runtime::MAKESLICE) + Call_expression* call = this->find_makeslice_call(sve->valmem()); + if (call != NULL + && Node::make_node(call)->encoding() == Node::ESCAPE_NONE) { Expression* len_arg = call->args()->at(1); Expression* cap_arg = call->args()->at(2); @@ -164,8 +188,7 @@ Mark_address_taken::expression(Expression** pexpr) && nclen.to_unsigned_long(&vlen) == Numeric_constant::NC_UL_VALID && nccap.to_unsigned_long(&vcap) == Numeric_constant::NC_UL_VALID) { - // Turn it into a slice expression of an addressable array, - // which is allocated on stack. + // Stack allocate an array and make a slice value from it. Location loc = expr->location(); Type* elmt_type = expr->type()->array_type()->element_type(); Expression* len_expr = @@ -173,10 +196,12 @@ Mark_address_taken::expression(Expression** pexpr) Type* array_type = Type::make_array_type(elmt_type, len_expr); Expression* alloc = Expression::make_allocation(array_type, loc); alloc->allocation_expression()->set_allocate_on_stack(); - Expression* array = Expression::make_unary(OPERATOR_MULT, alloc, loc); - Expression* zero = Expression::make_integer_ul(0, len_arg->type(), loc); - Expression* slice = - Expression::make_array_index(array, zero, len_arg, cap_arg, loc); + Type* ptr_type = Type::make_pointer_type(elmt_type); + Expression* ptr = Expression::make_unsafe_cast(ptr_type, alloc, + loc); + Expression* slice = + Expression::make_slice_value(expr->type(), ptr, len_arg, + cap_arg, loc); *pexpr = slice; } } diff --git a/libgo/go/runtime/pprof/mprof_test.go b/libgo/go/runtime/pprof/mprof_test.go index f428827232a..6fe892b46f4 100644 --- a/libgo/go/runtime/pprof/mprof_test.go +++ b/libgo/go/runtime/pprof/mprof_test.go @@ -45,7 +45,7 @@ func allocatePersistent1K() { // Allocate transient memory using reflect.Call. func allocateReflectTransient() { - memSink = make([]byte, 2<<20) + memSink = make([]byte, 3<<20) } func allocateReflect() { @@ -106,7 +106,7 @@ func TestMemoryProfiler(t *testing.T) { // GC means that sometimes the value is not collected. fmt.Sprintf(`(0|%v): (0|%v) \[%v: %v\] @( 0x[0-9,a-f]+)+ # 0x[0-9,a-f]+ pprof\.allocateReflectTransient\+0x[0-9,a-f]+ .*/mprof_test.go:48 -`, memoryProfilerRun, (2<<20)*memoryProfilerRun, memoryProfilerRun, (2<<20)*memoryProfilerRun), +`, memoryProfilerRun, (3<<20)*memoryProfilerRun, memoryProfilerRun, (3<<20)*memoryProfilerRun), } for _, test := range tests { diff --git a/libgo/go/runtime/slice.go b/libgo/go/runtime/slice.go index 2e874cc2fbc..7f9db4efa9e 100644 --- a/libgo/go/runtime/slice.go +++ b/libgo/go/runtime/slice.go @@ -61,7 +61,7 @@ func panicmakeslicecap() { panic(errorString("makeslice: cap out of range")) } -func makeslice(et *_type, len, cap int) slice { +func makeslice(et *_type, len, cap int) unsafe.Pointer { // NOTE: The len > maxElements check here is not strictly necessary, // but it produces a 'len out of range' error instead of a 'cap out of range' error // when someone does make([]T, bignumber). 'cap out of range' is true too, @@ -76,11 +76,10 @@ func makeslice(et *_type, len, cap int) slice { panicmakeslicecap() } - p := mallocgc(et.size*uintptr(cap), et, true) - return slice{p, len, cap} + return mallocgc(et.size*uintptr(cap), et, true) } -func makeslice64(et *_type, len64, cap64 int64) slice { +func makeslice64(et *_type, len64, cap64 int64) unsafe.Pointer { len := int(len64) if int64(len) != len64 { panicmakeslicelen() -- 2.30.2