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

[Chris Lattner via Phabricator via cfe-commits]

lattner added inline comments.


================
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),
----------------
craig.topper wrote:
> This could use DAG.getNOT if you're willing to make that change.
I'd prefer to keep this one separate, it isn't related to APInt.


================
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;
----------------
craig.topper wrote:
> I think we have DAG.getAllOnesConstant if you want to use it
Will do this in a followup


================
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;
----------------
craig.topper wrote:
> I think we have DAG.getAllOnesConstant if you want to use it
Likewise


================
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);
----------------
craig.topper wrote:
> That's a strange way to write APInt(64, 1) unless there was some specific bug.
I agree, assume that this was testing some former bug.  Unit tests are designed to be weird ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109483