[PATCH] D14945: Expose isXxxConstant() functions from TargetLowering base class (NFC)

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 25 11:02:50 PST 2015


MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

LGTM, good to commit with the nitpicks below addressed. Thanks for working on this!


================
Comment at: include/llvm/CodeGen/SelectionDAGNodes.h:1497-1500
@@ -1496,1 +1496,6 @@
 
+bool isNullConstant(SDValue V);
+bool isNullFPConstant(SDValue V);
+bool isAllOnesConstant(SDValue V);
+bool isOneConstant(SDValue V);
+
----------------
Even though this whole file lacks a lot of doxygen comments, we can do better when we add code. Please add some simple comments similar to
```
/// Returns true if \p V is a ConstantSDNode with value 0.
```

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:13885-13887
@@ -13897,8 +13884,5 @@
                                    SDLoc dl, SelectionDAG &DAG) const {
-  if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(Op1)) {
-    if (C->getAPIntValue() == 0)
+  if (isNullConstant(Op1)) {
       return EmitTest(Op0, X86CC, dl, DAG);
-
-     assert(Op0.getValueType() != MVT::i1 &&
-            "Unexpected comparison operation for MVT::i1 operands");
   }
+  assert(!(isa<ConstantSDNode>(Op1) && Op0.getValueType() == MVT::i1) &&
----------------
Does not need {} any longer.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:14851-14853
@@ -14881,3 +14850,5 @@
+      Cond.getOperand(0).getOpcode() == X86ISD::SETCC_CARRY &&
+      isOneConstant(Cond.getOperand(1))) {
       Cond = Cond.getOperand(0);
   }
 
----------------
Does not need {} any longer.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:15336-15339
@@ -15363,7 +15335,6 @@
     return false;
-  ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(Op.getOperand(1));
-  if (N1C && N1C->getAPIntValue() == 1) {
+  if (isOneConstant(Op.getOperand(1))) {
     return Op.getOperand(0).getOpcode() == X86ISD::SETCC &&
       Op.getOperand(0).hasOneUse();
   }
   return false;
----------------
Does not need {} any longer.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:15381-15385
@@ -15410,8 +15380,7 @@
   // Look pass (and (setcc_carry (cmp ...)), 1).
   if (Cond.getOpcode() == ISD::AND &&
-      Cond.getOperand(0).getOpcode() == X86ISD::SETCC_CARRY) {
-    ConstantSDNode *C = dyn_cast<ConstantSDNode>(Cond.getOperand(1));
-    if (C && C->getAPIntValue() == 1)
+      Cond.getOperand(0).getOpcode() == X86ISD::SETCC_CARRY &&
+      isOneConstant(Cond.getOperand(1))) {
       Cond = Cond.getOperand(0);
   }
 
----------------
Does not need {} any longer.


http://reviews.llvm.org/D14945





More information about the llvm-commits mailing list