[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.
lattner added a comment. > What is a "keep constructor"? Good question, I'm not sure. I think I meant to say "key constructors". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109483/new/ https://reviews.llvm.org/D109483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.
foad added a comment. What is a "keep constructor"? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109483/new/ https://reviews.llvm.org/D109483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.
lattner added a comment. I'll take care of the DAG.getAllOnesConstant change, but i'd appreciate it if you could look at the NOT cases. Running tests on the DAG.getAllOnesConstant patch now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109483/new/ https://reviews.llvm.org/D109483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.
lattner added a comment. Thank you for the detailed review Craig! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109483/new/ https://reviews.llvm.org/D109483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.
craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109483/new/ https://reviews.llvm.org/D109483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.
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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.
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 <> 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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.
lattner marked an inline comment as done. lattner added inline comments. Comment at: llvm/include/llvm/ADT/APInt.h:384 /// value for the APInt's bit width. bool isMaxValue() const { return isAllOnesValue(); } craig.topper wrote: > isAllOnes()? Yep, good catch, fixed thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109483/new/ https://reviews.llvm.org/D109483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.
craig.topper added inline comments. Comment at: llvm/include/llvm/ADT/APInt.h:384 /// value for the APInt's bit width. bool isMaxValue() const { return isAllOnesValue(); } isAllOnes()? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109483/new/ https://reviews.llvm.org/D109483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.
lattner added a comment. This patch has a lot of noise, the important part is APInt.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109483/new/ https://reviews.llvm.org/D109483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits