[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#1137930, @alexandre.isoard wrote:

> In https://reviews.llvm.org/D48338#1137890, @timshen wrote:
>
> > What about `zext(%a + %b) + %c`? I think `zext(%a) + zext(%b) + %c` is in a much better state than, idk, `zext(%a + %b + trunc(%c))`.
>
>
> I don't think those transformations are legal in the general case. But I would go for the expression that has the minimum bit-width of operators in general (that is, sink `trunc` but lift `sext` and `zext`).


True.

> Could you explain why your form is preferable (you may have good reasons I am missing)? I have a feeling the true issue is that we have trouble manipulating `zext`/`sext`/`trunc` and that is probably why we have some heuristic to not duplicate them too much.

There already exists transformation for `zext((A + B + ...)<nuw>) --> (zext(A) + zext(B) + ...)<nuw>` and `zext((A * B * ...)<nuw>) --> (zext(A) * zext(B) * ...)<nuw>`. I was to be consistent. The true is from my perspective is that a lot of pattern matching ignores potential zexts, therefore we want to push the zexts away (either to the root or to the leaves) so that pattern in the middle of the tree is not disturbed.

> I have a patch that improve the mobility of those on AddExpr, but I am not 100% sure of its correctness.
> 
> It transforms: `(sext i57 (199 * (trunc i64 (-1 + (2780916192016515319 * %n)) to i57)) to i64)` into `(sext i57 (-199 + (trunc i64 %n to i57)) to i64)` (not sure if that is correct).
> 
> Would that go towards your goal?

I'm specifically looking at the URem pattern, `zext(A-A/B*B)`. Nothing involves trunc.


https://reviews.llvm.org/D48338