PRE (sometimes) confuses ivopts/scev?

[Daniel Berlin]

On Tue, 25 Jan 2005, Ulrich Weigand wrote:

> Daniel Berlin wrote:
>
>> On Mon, 24 Jan 2005, Richard Guenther wrote:
>>
>>> Daniel Berlin wrote:
>>>>
>>>> Just as a followup, sebastian's patch is at (the actual patch is ta the
>>>> end of the message. You sahouldn't need the other two parts he's pasted in
>>>> front of it anymore, i believe)
>>>> http://gcc.gnu.org/ml/gcc-patches/2004-11/msg02444.html
>>>
>>> This patch applied, but didn't help, I'll try to get more details and
>>> hopefully a reduced testcase.
>>>
>>>> it should be applied because it helps us detect more simple evolutions
>>>> properly.
>>>> " during a bootstrap
>>>> there are about 7000 cases that are in fact just simple affine
>>>> evolution functions, but that are mishandled for the moment because
>>>> the scalar evolution analyzer was not enough smart to transform them."
>>>>
>>>> In addition, the patch to teach PRE to stop creating *more* wraparounds is
>>>> also in the mail archives, see
>>>> http://gcc.gnu.org/ml/gcc-patches/2004-11/msg02222.html
>>>
>>> This patch doesn't apply any more.
>>
>> Yes, i know.
>> The attached should apply.
>
> I'm also seeing the effect of PRE generating wrap-around variables
> in my mgrid test case.  It causes the final loop to look like:
>
> __label_000600:;
>  D.922 = (<unnamed type>) i1;
>  D.464 = (int8) (int4) (D.922 + 4294967295);
>  i1 = (int4) (D.922 + 1);
>  D.469 = (int8) i1;
>  prephitmp.55 = offset.11 + D.469 + prephitmp.32;
>  D.507 = (*v)[prephitmp.131] - D.453 * (*u)[prephitmp.131] - D.462 * ((*u)[offset.11 + D.464 + prephitmp.32] + (*u)[prephitmp.55] + (*u)[prephitmp.67] + (*u)[prephitmp.79] + (*u)[prephitmp.83] + (*u)[prephitmp.87]);
>  prephitmp.67 = offset.11 + D.469 + pretmp.62;
>  prephitmp.79 = offset.11 + D.469 + prephitmp.132;
>  prephitmp.83 = offset.11 + D.469 + prephitmp.129;
>  prephitmp.87 = offset.11 + D.469 + prephitmp.130;
>  D.561 = D.507 - D.508 * ((*u)[offset.11 + D.464 + pretmp.62] + (*u)[prephitmp.67] + (*u)[offset.11 + D.464 + prephitmp.132] + (*u)[prephitmp.79] + (*u)[prephitmp.93] + (*u)[prephitmp.99] + (*u)[prephitmp.105] + (*u)[prephitmp.111] + (*u)[offset.11 + D.464 + prephitmp.129] + (*u)[offset.11 + D.464 + prephitmp.130] + (*u)[prephitmp.83] + (*u)[prephitmp.87]);
>  prephitmp.93 = offset.11 + D.469 + pretmp.88;
>  prephitmp.99 = offset.11 + D.469 + prephitmp.40;
>  prephitmp.105 = offset.11 + D.469 + pretmp.100;
>  prephitmp.111 = offset.11 + D.469 + prephitmp.48;
>  (*r)[prephitmp.131] = D.561 - D.562 * ((*u)[offset.11 + D.464 + pretmp.88] + (*u)[prephitmp.93] + (*u)[offset.11 + D.464 + prephitmp.40] + (*u)[prephitmp.99] + (*u)[offset.11 + D.464 + pretmp.100] + (*u)[prephitmp.105] + (*u)[offset.11 + D.464 + prephitmp.48] + (*u)[prephitmp.111]);
>  if (D.412 == (int4) ((<unnamed type>) i1 + 4294967295)) goto <L8>; else goto <L29>;
>
> All those prephitmp variables try to reuse a value from the previous
> loop iteration in the next one, which only succeeds in creating
> enormous reigster pressure ...

As I said, this is a class of transformations that is never profitable, 
hence the reason i've got a patch to make it stop doing that.
However, nobody ever spoke up when i offered it for 4.0 :)


>
>
> Unfortunately your patch doesn't change this because the variables
> fail the
>
>      if (firstinsideloop ^ secondinsideloop
>          && is_gimple_min_invariant (avail[outsideloopblock->index])
>          && !is_gimple_min_invariant (avail[insideloopblock->index]))
>
> test -- the outsideloop value is not invariant (because it varies
> with the next outer loop).

> Why is this test required in the first place?

Because induction variables should be 2 argument phi nodes with the 
following property:

1. Edge incoming to loop doesn't change in the loop
2. Edge from inside loop changes.

This corresponds to an initial value, and an increment.




phi nodes where one 
value is 
incoming to the loop, and that value should not change *inside* the 
loop.
(and that value should be invariant in the loop)

  If I remove it,

Remove what?

If you remove the test and replace it with 1, it'll never PRE anything.