[PATCH] D45733: [DAGCombiner] Unfold scalar masked merge if profitable

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:5381-5421
+  auto matchD = [](SDValue D, SDValue Y) -> llvm::Optional<SDValue> /*X*/ {
+    if (D.getOpcode() != ISD::XOR)
+      return llvm::None;
+    SDValue D0 = D->getOperand(0);
+    SDValue D1 = D->getOperand(1);
+    if (D1 == Y)
+      return D0;
----------------
spatel wrote:
> After stepping through more of your tests, I see why this is ugly.
> 
> We don't have to capture the intermediate values if the hasOneUse() checks are in the lambda(s) though. What do you think of this version:
> 
> ```
>   // There are 3 commutable operators in the pattern, so we have to deal with
>   // 8 possible variants of the basic pattern.
>   SDValue X, Y, M;
>   auto matchAndXor = [&X,&Y,&M](SDValue And, unsigned XorIdx, SDValue Other) {
>     if (And.getOpcode() != ISD::AND || !And.hasOneUse())
>       return false;
>     if (And.getOperand(XorIdx).getOpcode() != ISD::XOR ||
>         !And.getOperand(XorIdx).hasOneUse())
>       return false;
>     SDValue Xor0 = And.getOperand(XorIdx).getOperand(0);
>     SDValue Xor1 = And.getOperand(XorIdx).getOperand(1);
>     if (Other == Xor0) std::swap(Xor0, Xor1);
>     if (Other != Xor1) return false;
>     X = Xor0;
>     Y = Xor1;
>     M = And.getOperand(1);
>     return true;
>   };
>   if (!matchAndXor(A, 0, B) && !matchAndXor(A, 1, B) &&
>       !matchAndXor(B, 0, A) && !matchAndXor(B, 1, A))
>     return SDValue();
> 
> ```
Oops - forgot to swap the M capture:
  M = And.getOperand(1);

should be:
  M = And.getOperand(XorIdx ? 0 : 1);



Repository:
  rL LLVM

https://reviews.llvm.org/D45733