[PATCH] D20058: [SCEV] No-wrap flags are not propagated when folding "{S, +, X}+T ==> {S+T, +, X}"

[Sanjoy Das via llvm-commits]

sanjoy added a comment.

In http://reviews.llvm.org/D20058#436432, @iid_iunknown wrote:

> Could you elaborate on this part of your comment, please?
>
>   with `CHECK:` lines verifying that the `sext` instructions don't get mapped to `sext` SCEV expressions.
>   
>
> A piece from my modified test that exposes the problem:
>
>   for.body3:
>     %i.16 = phi i32 [ %inc5, %for.body3 ], [ %i.0.lcssa, %for.cond1.preheader ]
>     %sub = add nsw i32 %i.16, -2
>     %idxprom = sext i32 %sub to i64
>     %arrayidx = getelementptr inbounds [10 x i32], [10 x i32]* %arr, i64 0, i64 %idxprom
>
>
>




> The patch eliminates %idxprom and uses an introduced %indvars.iv for

>  array indexing. I am not sure this is something that can be well

>  tested with `opt -analyze -scalar-evolution`. Opt shows different

>  outputs with and w/o the patch, but these differences mainly relate to

>  moving the constants outside of 'sext'. E.g.:




> **Patched:**

> 

>   %idxprom = sext i32 %sub to i64

>     -->  {(-2 + (sext i32 %i.0.lcssa to i64))<nsw>,+,1}<nsw><%for.body3> U: [-2147483650,4294967304) S: [-2147483650,4294967304)		Exits: (-2 + (zext i32 (9 + (-1 * %i.0.lcssa)) to i64) + (sext i32 %i.0.lcssa to i64))

> 

> 

> **No patch:**

> 

>   %idxprom = sext i32 %sub to i64

>     -->  {(sext i32 (-2 + %i.0.lcssa) to i64),+,1}<nsw><%for.body3> U: [-2147483648,4294967306) S: [-2147483648,4294967306)		Exits: ((zext i32 (9 + (-1 * %i.0.lcssa)) to i64) + (sext i32 (-2 + %i.0.lcssa) to i64))

> 


This _is_ the difference I'm suggesting we should test for.  In the
unpatched case, SCEV has not been able to prove that `(-2 +
%i.0.lcssa)` won't sign overflow, so it cannot commute a sext into the
addition to transform `sext(-2 + %i.0.lcssa)` into `-2 +
sext(%i.0.lcssa)`.  With your change SCEV is able to prove that `-2 +
%i.0.lcssa` does not sign overflow, so the sext can be commuted into
the addition, simplifying the expression to `(-2 + (sext i32
%i.0.lcssa to i64))` as you can see.

> My idea was to check that %idxprom gets eliminated and the array is

>  indexed by an expression w/o 'sext'. This can be done by `opt

>  -indvars` (`-analyze` is not useful for `-indvars` as

>  IndVarsSimplify::print() is not defined).


That's a useful test too, and if you're more comfortable with that I
won't be opposed to it.  But checking SCEV directly is more targeted.


Repository:
  rL LLVM

http://reviews.llvm.org/D20058