public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Go patch committed: Don't duplicate calls with multiple results
@ 2016-09-27 21:41 Ian Lance Taylor
  0 siblings, 0 replies; only message in thread
From: Ian Lance Taylor @ 2016-09-27 21:41 UTC (permalink / raw)
  To: gcc-patches, gofrontend-dev

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

While moving expressions around to preserve the specified order of
evaluation, the Go frontend was duplicating call expressions with
multiple results.  This had no bad effect as the second instance
generated no code for the backend.  But, it's pointless and wasteful.
This patch by Than McIntosh fixes the problem.  This fixes
https://golang.org/issue/17237.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian

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

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 240558)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@
-1d8d834b5eb9f683cc06529145b353bb5b08e7ea
+8aca265d317059ae6d9721a4a231895d80d0a82c
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/gogo.cc
===================================================================
--- gcc/go/gofrontend/gogo.cc	(revision 240453)
+++ gcc/go/gofrontend/gogo.cc	(working copy)
@@ -3616,11 +3616,21 @@ Order_eval::statement(Block* block, size
 	  // be handled specially.  We can't create a temporary
 	  // because there is no type to give it.  Any actual uses of
 	  // the values will be done via Call_result_expressions.
-	  s = Statement::make_statement(*pexpr, true);
-	}
+          //
+          // Since a given call expression can be shared by multiple
+          // Call_result_expressions, avoid hoisting the call the
+          // second time we see it here.
+          if (this->remember_expression(*pexpr))
+            s = NULL;
+          else
+            s = Statement::make_statement(*pexpr, true);
+        }
 
-      block->insert_statement_before(*pindex, s);
-      ++*pindex;
+      if (s != NULL)
+        {
+          block->insert_statement_before(*pindex, s);
+          ++*pindex;
+        }
     }
 
   if (init != orig_init)
@@ -7949,13 +7959,14 @@ Traverse::remember_type(const Type* type
 }
 
 // Record that we are looking at an expression, and return true if we
-// have already seen it.
+// have already seen it. NB: this routine used to assert if the traverse
+// mask did not include expressions/types -- this is no longer the case,
+// since it can be useful to remember specific expressions during
+// walks that only cover statements.
 
 bool
 Traverse::remember_expression(const Expression* expression)
 {
-  go_assert((this->traverse_mask() & traverse_types) != 0
-	     || (this->traverse_mask() & traverse_expressions) != 0);
   if (this->expressions_seen_ == NULL)
     this->expressions_seen_ = new Expressions_seen();
   std::pair<Expressions_seen::iterator, bool> ins =

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2016-09-27 21:25 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-27 21:41 Go patch committed: Don't duplicate calls with multiple results Ian Lance Taylor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).