[PATCH] D150922: [KnownBits] Return `0` for poison {s,u}div inputs

Noah Goldstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 19 14:06:35 PDT 2023


goldstein.w.n added a comment.

In D150922#4357670 <https://reviews.llvm.org/D150922#4357670>, @nikic wrote:

> In D150922#4357610 <https://reviews.llvm.org/D150922#4357610>, @goldstein.w.n wrote:
>
>> In D150922#4357344 <https://reviews.llvm.org/D150922#4357344>, @nikic wrote:
>>
>>> IMHO we shouldn't go out of the way to handle such cases -- if we need to handle this for other reasons (e.g. for shift amounts, or for conflicts) that's one thing, but explicitly checking for poison result for known constant cases is pretty pointless -- those will get folded to actual poison values by code that can do so.
>>
>> The INT_MIN / -1 case isn't necessary, but the LHS.isZero() || RHS.isZero() cases reduce the number of
>> checks we need elsewhere (specifically now we know MinTZ < BitWidth)
>>
>> Edit: But I have no particularly strong feelings either way. the Zero, Zero case just
>> seemed to be the simplest way to handle it. The INT_MIN, -1 case was just to
>> be consistent. If you feel either/both should be dropped I have no objections.
>
> Is this referring to the code in D150923 <https://reviews.llvm.org/D150923>? That is we do the early isZero() check here and then divComputeLowBit() from that patch has an implicit precondition that the denominator is not zero? Adding an explicit `< BitWidth` check there seems more obvious to me, unless it's more than that?

Need multiple (although I could just move this isZero() checks to `divComputeLowBit`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150922



More information about the llvm-commits mailing list