[PATCH] D112276: [InstCombine] Fold `(c & ~(a | b)) | (b & ~(a | c))` to `~a & (b ^ c)`

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

In D112276#3088380 <https://reviews.llvm.org/D112276#3088380>, @spatel wrote:

> In D112276#3085648 <https://reviews.llvm.org/D112276#3085648>, @rampitec wrote:
>
>> Added more tests:
>>
>> - or_not_and_commute4 - missing commute test. Note, I believe the rest of the commutes are covered because LHS and RHS are simmetrical. It is only a matter of what variables to call A, B and C, the logic is the same.
>
> If LHS and RHS are symmetric, then do we really need the second "match(Op1" clause in the code?

Yes, because in the match for Op0 we can have any order for OR operands. This commute is actually tested.

In D112276#3088399 <https://reviews.llvm.org/D112276#3088399>, @lebedev.ri wrote:

> In D112276#3088380 <https://reviews.llvm.org/D112276#3088380>, @spatel wrote:
>
>> I agree that we don't need one-use checks on everything. In fact, the match seems overspecified for that, so it might be limiting the optimization power in the motivating source. 
>> The way I view this is that we are creating 3 instructions, so as long as any 2 of the original intermediate values has one use, then the transform is desirable (because we're eliminating those 2 intermediate values + replacing the final `or` and reducing use counts of other values along the way).
>>
>> @lebedev.ri - do you have any suggestions for how to specify that constraint in the code?
>
> There is no nice way to do that, we are missing some common infra.
> What you want is to check that at least one of the hands of the outer `or` is single-use,
> and either the other hand is one-use either, or the `not` operand of the single-use hand is also single-use.

I can see an awkward way to do this, count a sum of `(int)hasOneUse()` expressions and compare to 7. Definitely do not like it.


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

https://reviews.llvm.org/D112276