[PATCH] D113783: [InstCombine] Fold (A^B)|~A-->~(A&B)

[Mehrnoosh Heidarpour via Phabricator via llvm-commits]

MehrHeidar added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2676
   // A | (~A ^ B) -> A | ~B
+  // ~A | (A ^ B) -> ~(A & B)
   // (A & B) | (A ^ B)
----------------
spatel wrote:
> This surprised me, but this seems to work. Note that 'not' is also an 'xor' operation.
> But canonicalization rules will always put the 'not' operand on the right (operand 1). So the swap above here then guarantees that the 'not' is operand 0.
Right, that is exactly the reason that I put the code after the `swap` in line `2671`.  If it is not appropriate I can change it.


================
Comment at: llvm/test/Transforms/InstCombine/or-xor.ll:62
 
+; ~X | (X ^ Y) --> ~(X & Y)
+
----------------
spatel wrote:
> We need another test where X and Y are commuted.
> We also need at least 3 tests with extra uses: (1) extra use of the 'not', (2) extra use of the regular 'xor', (3) extra uses of both of those.
> The transform should be fine as long there is only one use for either of the operands because we are eliminating an instruction in this sequence.
> When creating more tests, it's good to vary the data types a bit so we have better coverage. For example, have one test use vector types and another use a weird type like 'i65'.
Thank you; I have added a few more tests based on your suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113783