[PATCH] Fix PR35607, another revenge of invariant addresses (ivopts)

[Richard Guenther <rguenther at suse dot de>]

Occasionally we face "interesting" invariant addresses where at the
last time we chose to just allow them.  Now we hit another case
where IVOPTs choses to use an interesting one in a PHI node.  No
problem sofar, but this invariant address contains an SSA_NAME
(which we of course cannot create a use for there).

So, you say, how does this happen?  Easy.  We start from the invariant
expression

unsigned int i = (unsigned int) (((int) &__fini_array_end - (int)
&__fini_array_start) /[ex] 4);

which gimplification decomposes to

  __fini_array_end.0_2 = (int) &__fini_array_end;
  __fini_array_start.1_3 = (int) &__fini_array_start;
  D.1188_4 = __fini_array_end.0_2 - __fini_array_start.1_3;
  D.1189_5 = D.1188_4 /[ex] 4;
  i_6 = (unsigned int) D.1189_5;

of course not touching any of the TREE_INVARIANT flags.  So
(unsigned int) D.1189_5 is TREE_INVARIANT.  (neither of the
SSA_NAMEs have that flag set of course, just expression trees
that survived from their original state as given by the FE)

During producing a new address induction variable for

__fini_array_start [i_6]

IVOPTs expands "simple" operations, replacing i_6 by
(unsigned int) D.1189_5.  Now the result,
&__fini_array_start[(unsigned int) D.1189_5] is even
is_gimple_min_invariant(!) (even if you recompute that bit),
so the re-gimplification at edge-insertion time does nothing
and we end up with

# ivtmp.16_1 = PHI <ivtmp.16_11(5), &__fini_array_start[(unsigned int)
D.1189_5](3)>

which we don't like during verification (that part is actually a bug
caused by the early out for ADDR_EXPR and phi-nodes, as later on
we would to decide to not recurse into invariant ADDR_EXPRs because
we "allow" arbitrary complex ones.  Just that "fixing" that doesn't
help because of the SSA_NAME use in the PHI node).

So I tried two solutions, the first is to just fully expand all
invariant operations so that we end up with

# ivtmp.16_1 = PHI <ivtmp.16_11(5), &__fini_array_start[(unsigned int)
(((int) &__fini_array_end - (int) &__fini_array_start) /[ex] 4)](3)>

and "fix" the verifier.  This works fine until you try to build
libjava where you get interesting ICEs (I didn't try to debug that).

The other solution is to never expand TREE_INVARIANT operations that
are not gimple invariant (in this case, the conversion).  This
works fine apart from needing a testcase adjustment for loop-19.c
where if we vectorize we end up with

 vectmpa_3 = (vector double *)&a;
 MEM [base: vectmpa_3 ...

instead of the previous

 MEM [symbol: a ...

which IMHO is ok and it doesn't result in any asm differences.

So, bootstrapped and tested on x86_64-unknown-linux-gnu, applied to
mainline.

Now, maybe we should think about clearing the TREE_INVARIANT flag
during gimplification for everything that is not is_gimple_min_invariant.

Thanks,
Richard.

2008-03-16  Richard Guenther  <rguenther@suse.de>

	PR middle-end/35607
	* tree-ssa-loop-niter.c (expand_simple_operations): Do not
	expand TREE_INVARIANT operations that are not gimple invariant.

	* gcc.c-torture/compile/pr35607.c: New testcase.
	* gcc.dg/tree-ssa/loop-19.c: Use -O2 to avoid vectorization.

Index: testsuite/gcc.c-torture/compile/pr35607.c
===================================================================
*** testsuite/gcc.c-torture/compile/pr35607.c	(revision 0)
--- testsuite/gcc.c-torture/compile/pr35607.c	(revision 0)
***************
*** 0 ****
--- 1,9 ----
+ extern void (*__fini_array_start []) (void);
+ extern void (*__fini_array_end []) (void);
+ void
+ __libc_csu_fini (void)
+ {
+   __SIZE_TYPE__ i = __fini_array_end - __fini_array_start;
+   while (i-- > 0)
+     (*__fini_array_start [i]) ();
+ }
Index: tree-ssa-loop-niter.c
===================================================================
*** tree-ssa-loop-niter.c	(revision 133269)
--- tree-ssa-loop-niter.c	(working copy)
*************** expand_simple_operations (tree expr)
*** 1436,1441 ****
--- 1436,1445 ----
      return expr;
  
    e = GIMPLE_STMT_OPERAND (stmt, 1);
+   /* Do not expand random invariants.  */
+   if (TREE_INVARIANT (e)
+       && !is_gimple_min_invariant (e))
+     return expr;
    if (/* Casts are simple.  */
        TREE_CODE (e) != NOP_EXPR
        && TREE_CODE (e) != CONVERT_EXPR
Index: testsuite/gcc.dg/tree-ssa/loop-19.c
===================================================================
*** testsuite/gcc.dg/tree-ssa/loop-19.c	(revision 133269)
--- testsuite/gcc.dg/tree-ssa/loop-19.c	(working copy)
***************
*** 6,12 ****
  
  /* { dg-do compile { target i?86-*-* x86_64-*-* powerpc*-*-*} } */
  /* { dg-require-effective-target nonpic } */
! /* { dg-options "-O3 -fdump-tree-final_cleanup" } */
  
  # define N      2000000
  static double   a[N],c[N];
--- 6,12 ----
  
  /* { dg-do compile { target i?86-*-* x86_64-*-* powerpc*-*-*} } */
  /* { dg-require-effective-target nonpic } */
! /* { dg-options "-O2 -fdump-tree-final_cleanup" } */
  
  # define N      2000000
  static double   a[N],c[N];