[PATCH] D114339: [InstCombine] simplify (~A | B) ^ A --> ~( A & B)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:3652
+  // (~A | B) ^ A --> ~(A & B)
+  if (match(&I, m_c_Xor(m_c_Or(m_OneUse(m_Not(m_Deferred(Op1))), m_Value(B)), m_Deferred(Op1))) && Op0->hasOneUse())
+    return BinaryOperator::CreateNot(Builder.CreateAnd(Op1, B));
----------------
I'm not sure how the matching logic or one-use logic works here. It seems dangerous to overwrite the Op0/Op1 locals.

If you want a conservative use check, just put that on the 'or' instruction - as long as we're eliminating that instruction, this transform is a win:

  // (~A | B) ^ A --> ~(A & B)
  if (match(Op0, m_OneUse(m_c_Or(m_Not(m_Specific(Op1)), m_Value(B)))))
    return BinaryOperator::CreateNot(Builder.CreateAnd(Op1, B));

  // A ^ (~A | B) --> ~(A & B)
  if (match(Op1, m_OneUse(m_c_Or(m_Not(m_Specific(Op0)), m_Value(B)))))
    return BinaryOperator::CreateNot(Builder.CreateAnd(Op0, B));

But I think we're missing a test where the 'not A' has another use. Please add and make sure that looks right. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114339