Go patch committed: Move slice construction to callers of makeslice

Go patch committed: Move slice construction to callers of makeslice

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

This patch to the Go frontend moves slice construction to callers of
makeslice.  This is the gccgo version of https://golang.org/cl/141822,
which was described as:

    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.

Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 16740 bytes --]

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 267658)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -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.
Index: gcc/go/gofrontend/escape.cc
===================================================================
--- gcc/go/gofrontend/escape.cc	(revision 267580)
+++ gcc/go/gofrontend/escape.cc	(working copy)
@@ -1737,6 +1737,16 @@ Escape_analysis_assign::expression(Expre
       }
       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
 	  // 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:
Index: gcc/go/gofrontend/expressions.cc
===================================================================
--- gcc/go/gofrontend/expressions.cc	(revision 267580)
+++ gcc/go/gofrontend/expressions.cc	(working copy)
@@ -7787,21 +7787,29 @@ Builtin_call_expression::lower_make(Stat
   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(Stat
       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_ba
       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*
   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<Bexpression*> 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_expressi
 {
   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_expressi
 }
 
 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.
Index: gcc/go/gofrontend/expressions.h
===================================================================
--- gcc/go/gofrontend/expressions.h	(revision 267580)
+++ gcc/go/gofrontend/expressions.h	(working copy)
@@ -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<Receive_expression, EXPRESSION_RECEIVE>(); }
 
+  // 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<Slice_value_expression, EXPRESSION_SLICE_VALUE>(); }
+
   // If this is a conditional expression, return the Conditional_expression
   // structure.  Otherwise, return NULL.
   Conditional_expression*
@@ -3955,6 +3962,56 @@ class Receive_expression : public Expres
   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
Index: gcc/go/gofrontend/runtime.def
===================================================================
--- gcc/go/gofrontend/runtime.def	(revision 267580)
+++ gcc/go/gofrontend/runtime.def	(working copy)
@@ -85,10 +85,10 @@ DEF_GO_RUNTIME(COMPLEX128_DIV, "__go_com
 
 // 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.
Index: gcc/go/gofrontend/wb.cc
===================================================================
--- gcc/go/gofrontend/wb.cc	(revision 267580)
+++ gcc/go/gofrontend/wb.cc	(working copy)
@@ -41,6 +41,9 @@ class Mark_address_taken : public Traver
   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* blo
   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(Expressio
     }
 
   // 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)
-    {
-      Call_expression* call = uce->expr()->call_expression();
-      if (call->fn()->func_expression() != NULL
-          && call->fn()->func_expression()->runtime_code() == Runtime::MAKESLICE)
+  Slice_value_expression* sve = expr->slice_value_expression();
+  if (sve != NULL)
+    {
+      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(Expressio
               && 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(Expressio
               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;
             }
         }
Index: libgo/go/runtime/pprof/mprof_test.go
===================================================================
--- libgo/go/runtime/pprof/mprof_test.go	(revision 267580)
+++ libgo/go/runtime/pprof/mprof_test.go	(working copy)
@@ -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 {
Index: libgo/go/runtime/slice.go
===================================================================
--- libgo/go/runtime/slice.go	(revision 267580)
+++ libgo/go/runtime/slice.go	(working copy)
@@ -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)
 		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()