[PATCH] D48338: [SCEV] Improve zext(A /u B) and zext(A % B)

[Tim Shen via Phabricator via llvm-commits]

timshen added a comment.

In https://reviews.llvm.org/D48338#1138713, @alexandre.isoard wrote:

> In https://reviews.llvm.org/D48338#1138675, @timshen wrote:
>
> > Though I can't easily reduce to a test case, the code I'm specifically looking at looks like the following:
> >
> >   %11 = udiv i32 %10, 112
> >   %12 = mul i32 %11, 112
> >   %13 = sub i32 %10, %12
> >   %14 = urem i32 %11, 112
> >   %15 = udiv i32 %10, 12544
> >   %16 = zext i32 %15 to i64
> >   %17 = zext i32 %14 to i64
> >   %18 = zext i32 %13 to i64
> >   %19 = getelementptr inbounds [128 x [112 x [112 x [64 x float]]]], [128 x [112 x [112 x [64 x float]]]] addrspace(1)* %ptr, i64 0, i64 %16, i64 %17, i64 %18, i64 %3
> >   
> >
> > The idea is that %10 is a flat index of %ptr, and the whole GEP should be equivalent to (in C) `&%ptr[%10]`. This already works for a case where there is no zexts and everything is i32. This patch makes it work with zexts.
>
>
> I see. Does it work with `128` instead of `112`? `urem` with powers of two generate `trunc` and `zext`.
>
> My issue is that this patch goes:
>
> - in the reverse direction of what I need to simplify `trunc`/`zext` in general (but maybe that means my approach is not the right approach)
> - increase the computational complexity of expressions (I suspect that will make us generate worse code but probably negligible on CPU unless we do vectorization)
>
>   But, some of our reduction rules were already going in the reverse direction (I had to change some of those to have smaller expressions in the end), so I guess my objective is not aligned with our SCEV reduction rule (which I am not sure have been "designed").


Personally I don't have strong opinions on how to transform SCEV expression, as long as it doesn't end up like another InstCombine or DAGCombiner. :) If you have high-level design discussion, maybe shoot an email to llvm-dev?


Repository:
  rL LLVM

https://reviews.llvm.org/D48338