[PATCH] D149918: [InstCombine] Add oneuse checks to shr + cmp constant folds.

[Amara Emerson via Phabricator via llvm-commits]

aemerson added a comment.

In D149918#4327542 <https://reviews.llvm.org/D149918#4327542>, @nikic wrote:

> Please add tests as @dmgreen suggested (and also for whatever pattern you saw in oggenc, if it's a different one) and precommit them. See https://llvm.org/docs/TestingGuide.html#precommit-workflow-for-tests if you are unfamiliar with the precommit workflow.
>
> I'm open to considering this, as ultimately what we care about in InstCombine is what form is most beneficial for further transforms. Blocking this in the multi-use case will lose us some folds that could have made use of the simplified condition, but your data seems to suggest that in practice we lose more by failing to match select patterns. The alternative is to try harder when matching those select patterns.
>
> However, if we want to do this change, you will first have to implement LVI support to effectively undo this when reasoning about condition ranges. That is, LVI needs to know how to determine the range of `X` given a condition of the form `(ashr X, C) pred C2`. This works automatically if such comparisons get folded, but requires special support if they don't.

I'm not familiar with LVI, but I plugged in:

  define i1 @ashrsgt_01_00(i4 %x, ptr %p) {
    %s = ashr i4 %x, 1
    %c = icmp sgt i4 %s, 0
    ret i1 %c
  }

into opt and even after instcombine folds that to a single icmp, LVI doesn't seem to know anything about %x, just that it's `overdefined`. I assume you mean something else?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149918