Patchwork Avoid setting TREE_ADDRESSABLE during MEM_REF gimplification

login
register
mail settings
Submitter Richard Guenther
Date Feb. 21, 2012, 12:34 p.m.
Message ID <alpine.LNX.2.00.1202211330110.4999@zhemvz.fhfr.qr>
Download mbox | patch
Permalink /patch/142298/
State New
Headers show

Comments

Richard Guenther - Feb. 21, 2012, 12:34 p.m.
When PRE inserts stmts it re-gimplifies them.  This causes spurious
TREE_ADDRESSABLE bits to be set on decls as the testcase shows.
It's not easy to fix with the recursive nature of the gimplifier,
and generally we cannot rely on the gimplifier predicates to
properly tell us whether we need to do anything (at least not without
changing them, which would be a good thing, but not at this time).

The following patch places a workaround at the point we gimplify
the MEM_REF address instead.

Ideally gimplify_addr_expr would not mark things addressable (but
the frontends should, and mostly do, already do that), and
gimplify_expr would have an early out,
if (gimple_test_f (*expr_p)) return GS_ALL_DONE;

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2012-02-21  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/52324
	* gimplify.c (gimplify_expr): When re-gimplifying expressions
	do not gimplify a MEM_REF address operand if it is already
	in suitable form.

	* gcc.dg/tree-ssa/ssa-lim-10.c: New testcase.

Patch

Index: gcc/gimplify.c
===================================================================
--- gcc/gimplify.c	(revision 184428)
+++ gcc/gimplify.c	(working copy)
@@ -7061,15 +7061,23 @@  gimplify_expr (tree *expr_p, gimple_seq
 	      ret = GS_OK;
 	      break;
 	    }
-	  ret = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
-			       is_gimple_mem_ref_addr, fb_rvalue);
-	  if (ret == GS_ERROR)
-	    break;
+	  /* Avoid re-gimplifying the address operand if it is already
+	     in suitable form.  Re-gimplifying would mark the address
+	     operand addressable.  Always gimplify when not in SSA form
+	     as we still may have to gimplify decls with value-exprs.  */
+	  if (!gimplify_ctxp || !gimplify_ctxp->into_ssa
+	      || !is_gimple_mem_ref_addr (TREE_OPERAND (*expr_p, 0)))
+	    {
+	      ret = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
+				   is_gimple_mem_ref_addr, fb_rvalue);
+	      if (ret == GS_ERROR)
+		break;
+	    }
 	  recalculate_side_effects (*expr_p);
 	  ret = GS_ALL_DONE;
 	  break;
 
-	  /* Constants need not be gimplified.  */
+	/* Constants need not be gimplified.  */
 	case INTEGER_CST:
 	case REAL_CST:
 	case FIXED_CST:
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-lim-10.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-lim-10.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-lim-10.c	(revision 0)
@@ -0,0 +1,31 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-lim1-details" } */
+
+int *l, *r;
+int test_func(void)
+{
+  int i;
+  int direction;
+  static int pos;
+
+  pos = 0;
+  direction = 1;
+
+  for ( i = 0; i <= 400; i++ )
+    {
+      if ( direction == 0 )
+	pos = l[pos];
+      else
+	pos = r[pos];
+
+      if ( pos == -1 )
+	{
+	  pos = 0;
+	  direction = !direction;
+	}
+    }
+  return i;
+}
+
+/* { dg-final { scan-tree-dump "Executing store motion of pos" "lim1" } } */
+/* { dg-final { cleanup-tree-dump "lim1" } } */