[PATCH] D149423: [ValueTracking] Use knownbits interface for determining if `div`/`rem` are safe to speculate

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

In D149423#4334277 <https://reviews.llvm.org/D149423#4334277>, @StephenFan wrote:

>> Short term fix is obviously revert (still building), but longer term
>> I guess we need an API for "is always speculatively executable"?
>> or something like that.
>> @nikic, any thoughts on how best to proceed in trying to safely
>> strengthen the `isSpeculativelyExecutable` case?
>
> I think strengthening the 'isSpeculativlyExecutable' is a bit conservative. I found that this bug is only exposed in CVP's narrowUDivOrURem, we can add some checks to avoid it.

I tend to agree that this issue leans more on `narrowUDivOrURem` but are these implicit assumptions elsewhere in the project? If the common usage of `isSpeculativelyExecutable` has come to mean that its speculatively executable even if the non-const operands change then it is a bug in the change to `isSpeculativelyExecutable`, not with `narrowUDivOrURem`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149423