[PATCH] D157312: [InstCombine] Add transforms for `(or/and (icmp eq/ne X,0),(icmp eq/ne X,Pow2OrZero))`

Noah Goldstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 25 14:18:37 PDT 2023


goldstein.w.n added a comment.

In D157312#4618244 <https://reviews.llvm.org/D157312#4618244>, @nikic wrote:

> In D157312#4618198 <https://reviews.llvm.org/D157312#4618198>, @goldstein.w.n wrote:
>
>> In D157312#4616620 <https://reviews.llvm.org/D157312#4616620>, @nikic wrote:
>>
>>> Not sure we should do this transform in IR. The original form seems to be a lot simpler to reason about than the new one -- e.g. if you have that as condition, it's obvious that %x can't be zero (as there's an explicit check for it). Afterwards, that would be fairly hard to determine.
>>
>> Really? We have a fair amount of logic for `X & Y eq/ne ...` and we do this fold already for constants (and similiar folds for non-constants). 
>> Also since its an equivilency and only relies on `Pow2OrZero` being a pow2orzero, theres no information loss.
>
> For constants this is turned into an `X & ~C eq 0` or `X & ~C ne 0` pattern. That //is// well understood due to the obvious KnownBits implications. `X & Y == X` not so much I think.

How about if we get D145424 <https://reviews.llvm.org/D145424>, D145425 <https://reviews.llvm.org/D145425>, and D145426 <https://reviews.llvm.org/D145426> in as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157312



More information about the llvm-commits mailing list