[PATCH] D96534: [SCEV] Use both known bits and sign bits when computing range of SCEV unknowns

Fri Feb 12 09:15:23 PST 2021

reames added a comment.

In D96534#2559301 <https://reviews.llvm.org/D96534#2559301>, @nikic wrote:

> Could you share what your motivating test case for this was (if there is one)? I'm not opposed to this change (compile-time impact is minor, and should not have any pathological cases), but the test changes here don't look super compelling either.

The motivating case which led me to stumble on this is an outer loop with a shift recurrence IV - specifically an ashr recurrence starting with a positive number - which is used to control the trip count of an inner loop.  Without the change, we report a spurious signed range on the outer IV which includes a large negative portion.

In terms of transformation, the primary effect is blocking ashr to lshr conversion and LFTR in the outer loop.  It also simply makes it much harder to tell what's going on when looking at the analysis output which is actually the direct motivation.

I'm also planning to further build on top of this FYI.  In particular, we can use the trip count of the outer loop (small) and the fact we're looking at recurrence to get much tighter ranges.  That's hard to do without this building block though.

> The main real improvement (where we're not just lining up signed and unsigned ranges) seems to be in increasing-or-decreasing-iv.ll, but the results there are still sub-optimal, and I think these can be made optimal by resolving the comment at https://github.com/llvm/llvm-project/blob/4348d8ab7f6aa0359d99e49c2498af4ba2767505/llvm/lib/Analysis/ScalarEvolution.cpp#L6096-L6097. I'm also wondering whether it would make sense to handle selects the same way as phis and combine the operand ranges.

I haven't looked at the select side of things much.  Your proposal seems to make sense on the surface, but this hasn't been relevant yet.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96534/new/

https://reviews.llvm.org/D96534