Re: [PR67891] drop is_gimple_reg test from set_parm_rtl

[Alexandre Oliva <aoliva@redhat.com>]

On Oct 12, 2015, Richard Biener <richard.guenther@gmail.com> wrote:

> On Sat, Oct 10, 2015 at 3:16 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
>> On Oct  9, 2015, Richard Biener <richard.guenther@gmail.com> wrote:
>> 
>>> Ok.  Note that I think emit_block_move shouldn't mess with the addressable flag.
>> 
>> I have successfully tested a patch that stops it from doing so,
>> reverting https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49429#c11 but
>> according to bugs 49429 and 49454, it looks like removing it would mess
>> with escape analysis introduced in r175063 for bug 44194.  The thread
>> that introduces the mark_addressable calls suggests some discomfort with
>> this solution, and even a suggestion that the markings should be
>> deferred past the end of expand, but in the end there was agreement to
>> go with it.  https://gcc.gnu.org/ml/gcc-patches/2011-06/msg01746.html

> Aww, indeed.  Of course the issue is that we don't track pointers to the
> stack introduced during RTL properly.

> Thanks for checking.  Might want to add a comment before that
> addressable setting now that you've done the archeology.

I decided to give the following approach a try instead.  The following
patch was regstrapped on x86_64-linux-gnu and i686-linux-gnu.
Ok to install?

Would anyone with access to hpux (pa and ia64 are both affected) give it
a spin?


defer mark_addressable calls during expand till the end of expand

From: Alexandre Oliva <aoliva@redhat.com>

for  gcc/ChangeLog

	* gimple-expr.c: Include hash-set.h and rtl.h.
	(mark_addressable_queue): New var.
	(mark_addressable): Factor actual marking into...
	(mark_addressable_1): ... this.  Queue it up during expand.
	(mark_addressable_2): New.
	(flush_mark_addressable_queue): New.
	* gimple-expr.h (flush_mark_addressable_queue): Declare.
	* cfgexpand.c: Include gimple-expr.h.
	(pass_expand::execute): Flush mark_addressable queue.
---
 gcc/cfgexpand.c   |    3 +++
 gcc/gimple-expr.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++--
 gcc/gimple-expr.h |    1 +
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index eaad859..a362e17 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "internal-fn.h"
 #include "tree-eh.h"
 #include "gimple-iterator.h"
+#include "gimple-expr.h"
 #include "gimple-walk.h"
 #include "cgraph.h"
 #include "tree-cfg.h"
@@ -6373,6 +6374,8 @@ pass_expand::execute (function *fun)
   /* We're done expanding trees to RTL.  */
   currently_expanding_to_rtl = 0;
 
+  flush_mark_addressable_queue ();
+
   FOR_BB_BETWEEN (bb, ENTRY_BLOCK_PTR_FOR_FN (fun)->next_bb,
 		  EXIT_BLOCK_PTR_FOR_FN (fun), next_bb)
     {
diff --git a/gcc/gimple-expr.c b/gcc/gimple-expr.c
index 2a6ba1a..db249a3 100644
--- a/gcc/gimple-expr.c
+++ b/gcc/gimple-expr.c
@@ -35,6 +35,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimplify.h"
 #include "stor-layout.h"
 #include "demangle.h"
+#include "hash-set.h"
+#include "rtl.h"
 
 /* ----- Type related -----  */
 
@@ -823,6 +825,50 @@ is_gimple_mem_ref_addr (tree t)
 		  || decl_address_invariant_p (TREE_OPERAND (t, 0)))));
 }
 
+/* Hold trees marked addressable during expand.  */
+
+static hash_set<tree> *mark_addressable_queue;
+
+/* Mark X as addressable or queue it up if called during expand.  */
+
+static void
+mark_addressable_1 (tree x)
+{
+  if (!currently_expanding_to_rtl)
+    {
+      TREE_ADDRESSABLE (x) = 1;
+      return;
+    }
+
+  if (!mark_addressable_queue)
+    mark_addressable_queue = new hash_set<tree>();
+  mark_addressable_queue->add (x);
+}
+
+/* Adaptor for mark_addressable_1 for use in hash_set traversal.  */
+
+bool
+mark_addressable_2 (tree const &x, void * ATTRIBUTE_UNUSED = NULL)
+{
+  mark_addressable_1 (x);
+  return false;
+}
+
+/* Mark all queued trees as addressable, and empty the queue.  To be
+   called right after clearing CURRENTLY_EXPANDING_TO_RTL.  */
+
+void
+flush_mark_addressable_queue ()
+{
+  gcc_assert (!currently_expanding_to_rtl);
+  if (mark_addressable_queue)
+    {
+      mark_addressable_queue->traverse<void*, mark_addressable_2> (NULL);
+      delete mark_addressable_queue;
+      mark_addressable_queue = NULL;
+    }
+}
+
 /* Mark X addressable.  Unlike the langhook we expect X to be in gimple
    form and we don't do any syntax checking.  */
 
@@ -838,7 +884,7 @@ mark_addressable (tree x)
       && TREE_CODE (x) != PARM_DECL
       && TREE_CODE (x) != RESULT_DECL)
     return;
-  TREE_ADDRESSABLE (x) = 1;
+  mark_addressable_1 (x);
 
   /* Also mark the artificial SSA_NAME that points to the partition of X.  */
   if (TREE_CODE (x) == VAR_DECL
@@ -849,7 +895,7 @@ mark_addressable (tree x)
     {
       tree *namep = cfun->gimple_df->decls_to_pointers->get (x);
       if (namep)
-	TREE_ADDRESSABLE (*namep) = 1;
+	mark_addressable_1 (*namep);
     }
 }
 
diff --git a/gcc/gimple-expr.h b/gcc/gimple-expr.h
index 3d1c89f..2917d2752c 100644
--- a/gcc/gimple-expr.h
+++ b/gcc/gimple-expr.h
@@ -52,6 +52,7 @@ extern bool is_gimple_asm_val (tree);
 extern bool is_gimple_min_lval (tree);
 extern bool is_gimple_call_addr (tree);
 extern bool is_gimple_mem_ref_addr (tree);
+extern void flush_mark_addressable_queue (void);
 extern void mark_addressable (tree);
 extern bool is_gimple_reg_rhs (tree);
 


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer