[PATCH] D11289: [X86] Widen the 'AND' mask if doing so shrinks the encoding size

[Chandler Carruth]

chandlerc added a comment.

Cool implementation. Some thoughts inline about structuring the code a bit better.


================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:2226-2230
@@ +2225,7 @@
+  case ISD::AND: {
+    SDValue N0 = Node->getOperand(0);
+    SDValue N1 = Node->getOperand(1);
+    // Try to shrink the encoding of an AND by setting additional bits in the
+    // mask.  It is only correct to do so if we know a priori that the other
+    // operand of the AND already has those bits set to zero.
+    if (ConstantSDNode *Cst = dyn_cast<ConstantSDNode>(N1)) {
----------------
At this point, I think it would be worth extracting this to a helper function. That will let you use early-exit to make everything easier to read IMO.

================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:2251-2269
@@ +2250,21 @@
+        if (Opc8 || Opc32) {
+          APInt Op0Zero, Op0One;
+          CurDAG->computeKnownBits(N0, Op0Zero, Op0One);
+          // Grow the mask using the known zero bits.
+          Op0Zero |= Val;
+          // Check to see if the mask can be efficiently encoded.
+          SDNode *NewNode = nullptr;
+          if (Opc8 && Op0Zero.isSignedIntN(8)) {
+            SDValue NewCst =
+                CurDAG->getTargetConstant(Op0Zero.getSExtValue(), dl, MVT::i8);
+            NewNode = CurDAG->getMachineNode(Opc8, dl, NVT, N0, NewCst);
+          } else if (Opc32 && Op0Zero.isSignedIntN(32)) {
+            SDValue NewCst =
+                CurDAG->getTargetConstant(Op0Zero.getSExtValue(), dl, MVT::i32);
+            NewNode = CurDAG->getMachineNode(Opc32, dl, NVT, N0, NewCst);
+          }
+          if (NewNode) {
+            ReplaceUses(SDValue(Node, 0), SDValue(NewNode, 0));
+            return nullptr;
+          }
+        }
----------------
I would put all of this into a lambda that you can call with the number of bits, the MVT, and the opcode. It should be easier to read, and make the above tests able to just return the call of the lambda each time they find a valid opcode.


http://reviews.llvm.org/D11289