[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

I think I read this patch too closely. I'll leave it up to you how much of this you want to do.



================
Comment at: llvm/include/llvm/IR/Constants.h:206
   /// Determine if the value is all ones.
   bool isMinusOne() const { return Val.isAllOnesValue(); }
 
----------------
isAllOnes()


================
Comment at: llvm/include/llvm/Transforms/InstCombine/InstCombiner.h:171
       TrueIfSigned = true;
       return RHS.isAllOnesValue();
     case ICmpInst::ICMP_SGT: // True if LHS s> -1
----------------
isAllOnes since you're already in the area


================
Comment at: llvm/include/llvm/Transforms/InstCombine/InstCombiner.h:174
       TrueIfSigned = false;
       return RHS.isAllOnesValue();
     case ICmpInst::ICMP_SGE: // True if LHS s>= 0
----------------
isAllOnes


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3243
            "Don't know how to expand this subtraction!");
-    Tmp1 = DAG.getNode(ISD::XOR, dl, VT, Node->getOperand(1),
-               DAG.getConstant(APInt::getAllOnesValue(VT.getSizeInBits()), dl,
-                               VT));
+    Tmp1 = DAG.getNode(
+        ISD::XOR, dl, VT, Node->getOperand(1),
----------------
This could use DAG.getNOT if you're willing to make that change.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:965
+      DAG.getConstant(APInt::getAllOnes(BitTy.getSizeInBits()), DL, MaskTy);
   SDValue NotMask = DAG.getNode(ISD::XOR, DL, MaskTy, Mask, AllOnes);
 
----------------
I think this could also be DAG.getNOT but I'm less sure.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1212
+      DAG.getConstant(APInt::getAllOnes(VT.getScalarSizeInBits()), DL, VT);
   SDValue NotMask = DAG.getNode(ISD::XOR, DL, VT, Mask, AllOnes);
 
----------------
This could be DAG.getNOT


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4021
       // (X & (C l>>/<< Y)) ==/!= 0  -->  ((X <</l>> Y) & C) ==/!= 0
       if (C1.isNullValue())
         if (SDValue CC = optimizeSetCCByHoistingAndByConstFromLogicalShift(
----------------
isZero()


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4030
       // all bits set:   (X | (Y<<32)) == -1 --> (X & Y) == -1
       bool CmpZero = N1C->getAPIntValue().isNullValue();
+      bool CmpNegOne = N1C->getAPIntValue().isAllOnes();
----------------
isNullValue() -> isZero(). I was going to say you could use N1C->isZero() but I think it would have to be N1C->isNullValue() because that is the ConstantSDNode interface.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:12185
     else
-      OtherOp = DAG.getConstant(APInt::getAllOnesValue(VT.getSizeInBits()), dl,
-                                VT);
+      OtherOp = DAG.getConstant(APInt::getAllOnes(VT.getSizeInBits()), dl, VT);
     return true;
----------------
I think we have DAG.getAllOnesConstant if you want to use it


================
Comment at: llvm/lib/Target/Lanai/LanaiISelLowering.cpp:1403
     else
-      OtherOp =
-          DAG.getConstant(APInt::getAllOnesValue(VT.getSizeInBits()), dl, VT);
+      OtherOp = DAG.getConstant(APInt::getAllOnes(VT.getSizeInBits()), dl, VT);
     return true;
----------------
I think we have DAG.getAllOnesConstant if you want to use it


================
Comment at: llvm/lib/Target/M68k/M68kISelLowering.cpp:1983
   Carry = DAG.getNode(M68kISD::ADD, DL, DAG.getVTList(CarryVT, MVT::i32), Carry,
                       DAG.getConstant(NegOne, DL, CarryVT));
 
----------------
This is also getAllOnesConstant


================
Comment at: llvm/unittests/ADT/APIntTest.cpp:29
 TEST(APIntTest, ShiftLeftByZero) {
-  APInt One = APInt::getNullValue(65) + 1;
+  APInt One = APInt::getZero(65) + 1;
   APInt Shl = One.shl(0);
----------------
That's a strange way to write APInt(64, 1) unless there was some specific bug.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109483/new/

https://reviews.llvm.org/D109483