Go patch committed: In composite literals use temps only for interfaces

Ian Lance Taylor iant@golang.org
Tue Jun 29 20:44:54 GMT 2021


On Tue, Jun 29, 2021 at 11:01 AM Ian Lance Taylor <iant@golang.org> wrote:
>
> This patch to the Go frontend reduces the number of temporaries that
> the compiler genrrates for composite literals.  For a composite
> literal we only need to introduce a temporary variable if we may be
> converting to an interface type, so only do it then.  This saves over
> 80% of compilation time when using gccgo to compile
> cmd/internal/obj/x86, as the GCC middle-end spends a lot of time
> pointlessly computing interactions between temporary variables (GCC PR
> 101064).  This is for that PR and for https://golang.org/issue/46600.
> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> to mainline and GCC 11 branch.

Following up on this, this patch, for mainline only, stops generating
temporaries for composite literals at all.  After the change above we
were generating temporaries for composite literals when a conversion
to interface type was required.  However, Cherry's
https://golang.org/cl/176459 changed the compiler to insert explicit
type conversions.  And those explicit type conversions insert the
required temporaries in Type_conversion_expression::do_flatten.  So in
practice the composite literal do_flatten methods would never insert
temporaries, as the values they see would always be multi_eval_safe.
So just remove the unnecessary do_flatten methods.  Bootstrapped and
ran Go testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
-------------- next part --------------
65047dc414205e433c3bcf2a200efcbab329e06b
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index f7bcc8c484a..ab1384d698b 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-cad187fe3aceb2a7d964b64c70dfa8c8ad24ce65
+01cb2b5e69a2d08ef3cc1ea023c22ed9b79f5114
 
 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 94342b2f9b8..a0472acc209 100644
--- a/gcc/go/gofrontend/expressions.cc
+++ b/gcc/go/gofrontend/expressions.cc
@@ -15147,48 +15147,6 @@ Struct_construction_expression::do_copy()
   return ret;
 }
 
-// Flatten a struct construction expression.  Store the values into
-// temporaries if they may need interface conversion.
-
-Expression*
-Struct_construction_expression::do_flatten(Gogo*, Named_object*,
-					   Statement_inserter* inserter)
-{
-  if (this->vals() == NULL)
-    return this;
-
-  // If this is a constant struct, we don't need temporaries.
-  if (this->is_constant_struct() || this->is_static_initializer())
-    return this;
-
-  Location loc = this->location();
-  const Struct_field_list* fields = this->type_->struct_type()->fields();
-  Struct_field_list::const_iterator pf = fields->begin();
-  for (Expression_list::iterator pv = this->vals()->begin();
-       pv != this->vals()->end();
-       ++pv, ++pf)
-    {
-      go_assert(pf != fields->end());
-      if (*pv != NULL)
-	{
-          if ((*pv)->is_error_expression() || (*pv)->type()->is_error_type())
-            {
-              go_assert(saw_errors());
-              return Expression::make_error(loc);
-            }
-	  if (pf->type()->interface_type() != NULL
-	      && !(*pv)->is_multi_eval_safe())
-	    {
-	      Temporary_statement* temp =
-		Statement::make_temporary(NULL, *pv, loc);
-	      inserter->insert(temp);
-	      *pv = Expression::make_temporary_reference(temp, loc);
-	    }
-	}
-    }
-  return this;
-}
-
 // Make implicit type conversions explicit.
 
 void
@@ -15451,55 +15409,6 @@ Array_construction_expression::do_check_types(Gogo*)
     }
 }
 
-// Flatten an array construction expression.  Store the values into
-// temporaries if they may need interface conversion.
-
-Expression*
-Array_construction_expression::do_flatten(Gogo*, Named_object*,
-					   Statement_inserter* inserter)
-{
-  if (this->is_error_expression())
-    {
-      go_assert(saw_errors());
-      return this;
-    }
-
-  if (this->vals() == NULL)
-    return this;
-
-  // If this is a constant array, we don't need temporaries.
-  if (this->is_constant_array() || this->is_static_initializer())
-    return this;
-
-  // If the array element type is not an interface type, we don't need
-  // temporaries.
-  if (this->type_->array_type()->element_type()->interface_type() == NULL)
-    return this;
-
-  Location loc = this->location();
-  for (Expression_list::iterator pv = this->vals()->begin();
-       pv != this->vals()->end();
-       ++pv)
-    {
-      if (*pv != NULL)
-	{
-          if ((*pv)->is_error_expression() || (*pv)->type()->is_error_type())
-            {
-              go_assert(saw_errors());
-              return Expression::make_error(loc);
-            }
-	  if (!(*pv)->is_multi_eval_safe())
-	    {
-	      Temporary_statement* temp =
-		Statement::make_temporary(NULL, *pv, loc);
-	      inserter->insert(temp);
-	      *pv = Expression::make_temporary_reference(temp, loc);
-	    }
-	}
-    }
-  return this;
-}
-
 // Make implicit type conversions explicit.
 
 void
@@ -15768,14 +15677,14 @@ Slice_construction_expression::create_array_val()
 // the new temp statement.
 
 Expression*
-Slice_construction_expression::do_flatten(Gogo* gogo, Named_object* no,
+Slice_construction_expression::do_flatten(Gogo*, Named_object*,
                                           Statement_inserter* inserter)
 {
   if (this->type()->array_type() == NULL)
-    return NULL;
-
-  // Base class flattening first
-  this->Array_construction_expression::do_flatten(gogo, no, inserter);
+    {
+      go_assert(saw_errors());
+      return Expression::make_error(this->location());
+    }
 
   // Create a stack-allocated storage temp if storage won't escape
   if (!this->storage_escapes_
@@ -15784,7 +15693,7 @@ Slice_construction_expression::do_flatten(Gogo* gogo, Named_object* no,
     {
       Location loc = this->location();
       this->array_val_ = this->create_array_val();
-      go_assert(this->array_val_);
+      go_assert(this->array_val_ != NULL);
       Temporary_statement* temp =
           Statement::make_temporary(this->valtype_, this->array_val_, loc);
       inserter->insert(temp);
diff --git a/gcc/go/gofrontend/expressions.h b/gcc/go/gofrontend/expressions.h
index e3747cc65d7..57c974d3c70 100644
--- a/gcc/go/gofrontend/expressions.h
+++ b/gcc/go/gofrontend/expressions.h
@@ -3806,9 +3806,6 @@ class Struct_construction_expression : public Expression,
   Expression*
   do_copy();
 
-  Expression*
-  do_flatten(Gogo*, Named_object*, Statement_inserter*);
-
   Bexpression*
   do_get_backend(Translate_context*);
 
@@ -3881,9 +3878,6 @@ protected:
   indexes()
   { return this->indexes_; }
 
-  Expression*
-  do_flatten(Gogo*, Named_object*, Statement_inserter*);
-
   // Get the backend constructor for the array values.
   Bexpression*
   get_constructor(Translate_context* context, Btype* btype);


More information about the Gcc-patches mailing list