[PATCH] D45664: [InstCombine] Canonicalize variable mask in masked merge

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2495-2497
+  if (Instruction *Xor = invertMaskInMaskedMerge(I, Builder))
+    return Xor;
+
----------------
spatel wrote:
> Make sure I haven't missed anything, but it seems easier to match this inline here more simply as:
> 
>   // Eliminate a 'not' from xor form of masked-merge (aka, bitwise select).
>   // The 'not' must be operand 1 of the 'and' (complexity-based op ordering).
>   // ((Op1 ^ X) & ~Y) ^ Op1 --> ((Op1 ^ X) & Y) ^ X
>   if (match(Op0, m_OneUse(m_And(m_c_Xor(m_Specific(Op1), m_Value(X)),
>                                 m_Not(m_Value(Y)))))) {
>     Value *Op00 = cast<BinaryOperator>(Op0)->getOperand(0);
>     return BinaryOperator::CreateXor(Builder.CreateAnd(Op00, Y), X);
>   }
>   // Op0 ^ ((Op0 ^ X) & ~Y) --> ((Op0 ^ X) & Y) ^ X
>   if (match(Op1, m_OneUse(m_And(m_c_Xor(m_Specific(Op0), m_Value(X)),
>                                 m_Not(m_Value(Y)))))) {
>     Value *Op10 = cast<BinaryOperator>(Op1)->getOperand(0);
>     return BinaryOperator::CreateXor(Builder.CreateAnd(Op10, Y), X);
>   }
> 
I'm sorry, i don't really like this, especially after seeing the DAGCombiner :)
I'm going to work on fixing the commutability problem of `BinaryOp_match` and `m_Specific()`.
(if that fix won't be accepted, will revisit this.)


Repository:
  rL LLVM

https://reviews.llvm.org/D45664