[PATCH] D46494: [DAGCombiner] Masked merge: enhance handling of 'andn' with immediates

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:5415-5416
 
-  SDValue A = N->getOperand(0);
-  SDValue B = N->getOperand(1);
-  if (!matchAndXor(A, 0, B) && !matchAndXor(A, 1, B) && !matchAndXor(B, 0, A) &&
-      !matchAndXor(B, 1, A))
+  SDValue A_ = N->getOperand(0);
+  SDValue B_ = N->getOperand(1);
+  if (!matchAndXor(A_, 0, B_) && !matchAndXor(A_, 1, B_) &&
----------------
I'd just call these 'N0' and 'N1' to avoid confusion with the matched values.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:5437
+    assert(TLI.hasAndNot(X) && "Only mask is a variable? Unreachable.");
+    // If not, de-canonicalze (Invert) the mask, swap the value in B part.
+    SDValue NewA = DAG.getNode(ISD::AND, DL, VT, D, NotM);
----------------
typo: canonicalize


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:5439
+    SDValue NewA = DAG.getNode(ISD::AND, DL, VT, D, NotM);
+    return DAG.getNode(ISD::OR, DL, VT, NewA, X);
+  }
----------------
How did this become an 'or'?
https://rise4fun.com/Alive/UMY

We should have a comment/formula to describe the overall transform and why it makes sense:
  // If we can't form an 'andn' with Y (because it's a constant), 
  // swap the final xor operand, so we can still use 'andn' to invert the mask:
  // ((X ^ C) & M) ^ C -->  ((X ^ C) & NotM') ^ X


Repository:
  rL LLVM

https://reviews.llvm.org/D46494