[PATCH] D48853: [SCEV] Add zext(C + x + ...) -> D + zext(C-D + x + ...)<nuw> transform

Roman Tereshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 17 15:25:09 PDT 2018


rtereshin added a comment.

In https://reviews.llvm.org/D48853#1165748, @efriedma wrote:

> I think if we're going to do this, we need to implement it on top of a SCEV-based known-bits implementation; introducing a separate getZeroExtendExprForValue API is going to lead to weird results if SCEV creates a zero-extend expression for some other reason.


I agree, that would be a more generic and homogeneous solution. Using `(ConstantRange, KnownBits)` pair instead of `(ConstantRange, minTrailingZeros)` (let alone only one component of the latter pair) across Scalar Evolution consistently may also benefit the framework in a number of other ways. It's a more intrusive change though. For now I think I could try to go with the number of trailing zeros approach despite the loss in generality if you or community feel strongly against known bits used the way the are used now in this patch.

> Whether we should do this in general, I'm not really sure.  I mean, yes, I can see how this particular form is a bit more convenient for the load-store vectorizer, but it doesn't seem very general; it seems more intuitive to canonicalize towards reducing the number of AddExprs.  But maybe pulling as much information as possible outside of the zext is generally useful enough to make this worthwhile?

I think this transformation reduces the number of possible operands of a `zext`, so it brings some of the expressions in `C1 + zext(C2 + X)` form to the same shape - often `C3 + zext(C4 * 2^k + X)` - which is canonicalization (if some of the constants are missing let's say they are just zeroes). There is `2^(2w)` different pairs of constants `(C1, C2)`, and only `2^(2w - k)` different pair of `(C3, C4 ^ 2^k)`, where `w` is the bit width of the type.

> Do you also plan to implement a similar transform for AddRecs? (e.g. `(zext i32 {1,+,2}<%while.body> to i64)`).

I suppose I should, what would you suggest?


Repository:
  rL LLVM

https://reviews.llvm.org/D48853





More information about the llvm-commits mailing list