This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Hi,

> 
> 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(!)

this seems to be the problem -- it obviously should not be
is_gimple_min_invariant.

> The other solution is to never expand TREE_INVARIANT operations that
> are not gimple invariant (in this case, the conversion).

Now this certainly is not the proper way to fix the problem -- why
should expand_simple_operations care?

> 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.

No, this certainly is not OK; it is a different, possibly more
expensive addressing mode.  Of course, in this case it will be fixed
by constant propagation on rtl, but what if such an addressing
mode does not exist -- i.e., what if there is reg + symbol(i.e., constant)
addressing mode, but not reg + reg addressing mode?

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

Could you please revert this "fix", at least until we agree that there
is no other cleaner way how to fix the problem?  Thanks,

Zdenek


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]