From f3ec47a007c6e8274a7895d8c3348caa78a3ca43 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 14 May 2025 10:13:30 -0400 Subject: [PATCH 01/12] [SelectionDAG] Introduce ISD::PTRADD This opcode represents the addition of a pointer value (first operand) and an integer offset (second operand). PTRADD nodes are only generated if the TargetMachine opts in by overriding TargetMachine::shouldPreservePtrArith(). The PTRADD node and respective visitPTRADD() function were adapted by @rgwott from the CHERI/Morello LLVM tree. Original authors: @davidchisnall, @jrtc27, @arichardson. The changes in this PR were extracted from PR #105669. Co-authored-by: Jessica Clarke Co-authored-by: Alexander Richardson Co-authored-by: Rodolfo Wottrich --- llvm/include/llvm/CodeGen/ISDOpcodes.h | 5 + llvm/include/llvm/Target/TargetMachine.h | 5 + .../include/llvm/Target/TargetSelectionDAG.td | 2 +- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 103 +++++++++++++++++- .../lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 10 +- .../SelectionDAG/SelectionDAGBuilder.cpp | 19 ++-- .../SelectionDAG/SelectionDAGDumper.cpp | 1 + 7 files changed, 129 insertions(+), 16 deletions(-) diff --git a/llvm/include/llvm/CodeGen/ISDOpcodes.h b/llvm/include/llvm/CodeGen/ISDOpcodes.h index 80ef32aff62ae..abae3e921117f 100644 --- a/llvm/include/llvm/CodeGen/ISDOpcodes.h +++ b/llvm/include/llvm/CodeGen/ISDOpcodes.h @@ -1502,6 +1502,11 @@ enum NodeType { // Outputs: [rv], output chain, glue PATCHPOINT, + // PTRADD represents pointer arithmatic semantics, for targets that opt in + // using shouldPreservePtrArith(). + // ptr = PTRADD ptr, offset + PTRADD, + // Vector Predication #define BEGIN_REGISTER_VP_SDNODE(VPSDID, ...) VPSDID, #include "llvm/IR/VPIntrinsics.def" diff --git a/llvm/include/llvm/Target/TargetMachine.h b/llvm/include/llvm/Target/TargetMachine.h index 906926729ed74..8536439f0a2d4 100644 --- a/llvm/include/llvm/Target/TargetMachine.h +++ b/llvm/include/llvm/Target/TargetMachine.h @@ -468,6 +468,11 @@ class TargetMachine { return false; } + /// True if target has some particular form of dealing with pointer arithmetic + /// semantics. False if pointer arithmetic should not be preserved for passes + /// such as instruction selection, and can fallback to regular arithmetic. + virtual bool shouldPreservePtrArith(const Function &F) const { return false; } + /// Create a pass configuration object to be used by addPassToEmitX methods /// for generating a pipeline of CodeGen passes. virtual TargetPassConfig *createPassConfig(PassManagerBase &PM) { diff --git a/llvm/include/llvm/Target/TargetSelectionDAG.td b/llvm/include/llvm/Target/TargetSelectionDAG.td index 41fed692c7025..349fd003ca06e 100644 --- a/llvm/include/llvm/Target/TargetSelectionDAG.td +++ b/llvm/include/llvm/Target/TargetSelectionDAG.td @@ -407,7 +407,7 @@ def tblockaddress: SDNode<"ISD::TargetBlockAddress", SDTPtrLeaf, [], def add : SDNode<"ISD::ADD" , SDTIntBinOp , [SDNPCommutative, SDNPAssociative]>; -def ptradd : SDNode<"ISD::ADD" , SDTPtrAddOp, []>; +def ptradd : SDNode<"ISD::PTRADD" , SDTPtrAddOp, []>; def sub : SDNode<"ISD::SUB" , SDTIntBinOp>; def mul : SDNode<"ISD::MUL" , SDTIntBinOp, [SDNPCommutative, SDNPAssociative]>; diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index d6e288a59b2ee..6129c1a89912e 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -412,7 +412,9 @@ namespace { SDValue visitMERGE_VALUES(SDNode *N); SDValue visitADD(SDNode *N); SDValue visitADDLike(SDNode *N); - SDValue visitADDLikeCommutative(SDValue N0, SDValue N1, SDNode *LocReference); + SDValue visitADDLikeCommutative(SDValue N0, SDValue N1, + SDNode *LocReference); + SDValue visitPTRADD(SDNode *N); SDValue visitSUB(SDNode *N); SDValue visitADDSAT(SDNode *N); SDValue visitSUBSAT(SDNode *N); @@ -1095,7 +1097,7 @@ bool DAGCombiner::reassociationCanBreakAddressingModePattern(unsigned Opc, // (load/store (add, (add, x, y), offset2)) -> // (load/store (add, (add, x, offset2), y)). - if (N0.getOpcode() != ISD::ADD) + if (N0.getOpcode() != ISD::ADD && N0.getOpcode() != ISD::PTRADD) return false; // Check for vscale addressing modes. @@ -1852,6 +1854,7 @@ SDValue DAGCombiner::visit(SDNode *N) { case ISD::TokenFactor: return visitTokenFactor(N); case ISD::MERGE_VALUES: return visitMERGE_VALUES(N); case ISD::ADD: return visitADD(N); + case ISD::PTRADD: return visitPTRADD(N); case ISD::SUB: return visitSUB(N); case ISD::SADDSAT: case ISD::UADDSAT: return visitADDSAT(N); @@ -2388,7 +2391,7 @@ static bool canFoldInAddressingMode(SDNode *N, SDNode *Use, SelectionDAG &DAG, } TargetLowering::AddrMode AM; - if (N->getOpcode() == ISD::ADD) { + if (N->getOpcode() == ISD::ADD || N->getOpcode() == ISD::PTRADD) { AM.HasBaseReg = true; ConstantSDNode *Offset = dyn_cast(N->getOperand(1)); if (Offset) @@ -2617,6 +2620,100 @@ SDValue DAGCombiner::foldSubToAvg(SDNode *N, const SDLoc &DL) { return SDValue(); } +/// Try to fold a pointer arithmetic node. +/// This needs to be done separately from normal addition, because pointer +/// addition is not commutative. +/// This function was adapted from DAGCombiner::visitPTRADD() from the Morello +/// project, which is based on CHERI. +SDValue DAGCombiner::visitPTRADD(SDNode *N) { + SDValue N0 = N->getOperand(0); + SDValue N1 = N->getOperand(1); + EVT PtrVT = N0.getValueType(); + EVT IntVT = N1.getValueType(); + SDLoc DL(N); + + // fold (ptradd undef, y) -> undef + if (N0.isUndef()) + return N0; + + // fold (ptradd x, undef) -> undef + if (N1.isUndef()) + return DAG.getUNDEF(PtrVT); + + // fold (ptradd x, 0) -> x + if (isNullConstant(N1)) + return N0; + + if (N0.getOpcode() == ISD::PTRADD && + !reassociationCanBreakAddressingModePattern(ISD::PTRADD, DL, N, N0, N1)) { + SDValue X = N0.getOperand(0); + SDValue Y = N0.getOperand(1); + SDValue Z = N1; + bool N0OneUse = N0.hasOneUse(); + bool YIsConstant = DAG.isConstantIntBuildVectorOrConstantInt(Y); + bool ZIsConstant = DAG.isConstantIntBuildVectorOrConstantInt(Z); + bool ZOneUse = Z.hasOneUse(); + + // (ptradd (ptradd x, y), z) -> (ptradd x, (add y, z)) if: + // * x is a null pointer; or + // * y is a constant and z has one use; or + // * y is a constant and (ptradd x, y) has one use; or + // * (ptradd x, y) and z have one use and z is not a constant. + if (isNullConstant(X) || (YIsConstant && ZOneUse) || + (YIsConstant && N0OneUse) || (N0OneUse && ZOneUse && !ZIsConstant)) { + SDValue Add = DAG.getNode(ISD::ADD, DL, IntVT, {Y, Z}); + + // Calling visit() can replace the Add node with ISD::DELETED_NODE if + // there aren't any users, so keep a handle around whilst we visit it. + HandleSDNode ADDHandle(Add); + + SDValue VisitedAdd = visit(Add.getNode()); + if (VisitedAdd) { + // If visit() returns the same node, it means the SDNode was RAUW'd, and + // therefore we have to load the new value to perform the checks whether + // the reassociation fold is profitable. + if (VisitedAdd.getNode() == Add.getNode()) + Add = ADDHandle.getValue(); + else + Add = VisitedAdd; + } + + return DAG.getMemBasePlusOffset(X, Add, DL, SDNodeFlags()); + } + + // TODO: There is another possible fold here that was proven useful. + // It would be this: + // + // (ptradd (ptradd x, y), z) -> (ptradd (ptradd x, z), y) if: + // * (ptradd x, y) has one use; and + // * y is a constant; and + // * z is not a constant. + // + // In some cases, specifically in AArch64's FEAT_CPA, it exposes the + // opportunity to select more complex instructions such as SUBPT and + // MSUBPT. However, a hypothetical corner case has been found that we could + // not avoid. Consider this (pseudo-POSIX C): + // + // char *foo(char *x, int z) {return (x + LARGE_CONSTANT) + z;} + // char *p = mmap(LARGE_CONSTANT); + // char *q = foo(p, -LARGE_CONSTANT); + // + // Then x + LARGE_CONSTANT is one-past-the-end, so valid, and a + // further + z takes it back to the start of the mapping, so valid, + // regardless of the address mmap gave back. However, if mmap gives you an + // address < LARGE_CONSTANT (ignoring high bits), x - LARGE_CONSTANT will + // borrow from the high bits (with the subsequent + z carrying back into + // the high bits to give you a well-defined pointer) and thus trip + // FEAT_CPA's pointer corruption checks. + // + // We leave this fold as an opportunity for future work, addressing the + // corner case for FEAT_CPA, as well as reconciling the solution with the + // more general application of pointer arithmetic in other future targets. + } + + return SDValue(); +} + /// Try to fold a 'not' shifted sign-bit with add/sub with constant operand into /// a shift and add with a different constant. static SDValue foldAddSubOfSignBit(SDNode *N, const SDLoc &DL, diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index bbf1b0fd590ef..1483a5f4d5b95 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -5641,7 +5641,8 @@ bool SelectionDAG::isADDLike(SDValue Op, bool NoWrap) const { bool SelectionDAG::isBaseWithConstantOffset(SDValue Op) const { return Op.getNumOperands() == 2 && isa(Op.getOperand(1)) && - (Op.getOpcode() == ISD::ADD || isADDLike(Op)); + (Op.getOpcode() == ISD::ADD || Op.getOpcode() == ISD::PTRADD || + isADDLike(Op)); } bool SelectionDAG::isKnownNeverNaN(SDValue Op, bool SNaN, @@ -8144,7 +8145,12 @@ SDValue SelectionDAG::getMemBasePlusOffset(SDValue Ptr, SDValue Offset, const SDNodeFlags Flags) { assert(Offset.getValueType().isInteger()); EVT BasePtrVT = Ptr.getValueType(); - return getNode(ISD::ADD, DL, BasePtrVT, Ptr, Offset, Flags); + if (!this->getTarget().shouldPreservePtrArith( + this->getMachineFunction().getFunction())) { + return getNode(ISD::ADD, DL, BasePtrVT, Ptr, Offset, Flags); + } else { + return getNode(ISD::PTRADD, DL, BasePtrVT, Ptr, Offset, Flags); + } } /// Returns true if memcpy source is constant data. diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 8e74a076cc013..b3227d9b589dd 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -4284,8 +4284,8 @@ void SelectionDAGBuilder::visitGetElementPtr(const User &I) { (int64_t(Offset) >= 0 && NW.hasNoUnsignedSignedWrap())) Flags |= SDNodeFlags::NoUnsignedWrap; - N = DAG.getNode(ISD::ADD, dl, N.getValueType(), N, - DAG.getConstant(Offset, dl, N.getValueType()), Flags); + N = DAG.getMemBasePlusOffset( + N, DAG.getConstant(Offset, dl, N.getValueType()), dl, Flags); } } else { // IdxSize is the width of the arithmetic according to IR semantics. @@ -4329,7 +4329,7 @@ void SelectionDAGBuilder::visitGetElementPtr(const User &I) { OffsVal = DAG.getSExtOrTrunc(OffsVal, dl, N.getValueType()); - N = DAG.getNode(ISD::ADD, dl, N.getValueType(), N, OffsVal, Flags); + N = DAG.getMemBasePlusOffset(N, OffsVal, dl, Flags); continue; } @@ -4389,7 +4389,7 @@ void SelectionDAGBuilder::visitGetElementPtr(const User &I) { SDNodeFlags AddFlags; AddFlags.setNoUnsignedWrap(NW.hasNoUnsignedWrap()); - N = DAG.getNode(ISD::ADD, dl, N.getValueType(), N, IdxN, AddFlags); + N = DAG.getMemBasePlusOffset(N, IdxN, dl, AddFlags); } } @@ -9138,8 +9138,8 @@ bool SelectionDAGBuilder::visitMemPCpyCall(const CallInst &I) { Size = DAG.getSExtOrTrunc(Size, sdl, Dst.getValueType()); // Adjust return pointer to point just past the last dst byte. - SDValue DstPlusSize = DAG.getNode(ISD::ADD, sdl, Dst.getValueType(), - Dst, Size); + SDNodeFlags Flags; + SDValue DstPlusSize = DAG.getMemBasePlusOffset(Dst, Size, sdl, Flags); setValue(&I, DstPlusSize); return true; } @@ -11230,10 +11230,9 @@ TargetLowering::LowerCallTo(TargetLowering::CallLoweringInfo &CLI) const { MachineFunction &MF = CLI.DAG.getMachineFunction(); Align HiddenSRetAlign = MF.getFrameInfo().getObjectAlign(DemoteStackIdx); for (unsigned i = 0; i < NumValues; ++i) { - SDValue Add = - CLI.DAG.getNode(ISD::ADD, CLI.DL, PtrVT, DemoteStackSlot, - CLI.DAG.getConstant(Offsets[i], CLI.DL, PtrVT), - SDNodeFlags::NoUnsignedWrap); + SDValue Add = CLI.DAG.getMemBasePlusOffset( + DemoteStackSlot, CLI.DAG.getConstant(Offsets[i], CLI.DL, PtrVT), + CLI.DL, SDNodeFlags::NoUnsignedWrap); SDValue L = CLI.DAG.getLoad( RetTys[i], CLI.DL, CLI.Chain, Add, MachinePointerInfo::getFixedStack(CLI.DAG.getMachineFunction(), diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp index 8faf97271d99e..45ef475dffe6e 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp @@ -269,6 +269,7 @@ std::string SDNode::getOperationName(const SelectionDAG *G) const { // Binary operators case ISD::ADD: return "add"; + case ISD::PTRADD: return "ptradd"; case ISD::SUB: return "sub"; case ISD::MUL: return "mul"; case ISD::MULHU: return "mulhu"; From ba0b2ee704eecba0825a332a3c0066bc1a2fa848 Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Thu, 15 May 2025 03:24:58 -0400 Subject: [PATCH 02/12] Use AddToWorklist() instead of manually re-visiting a newly generated node in the DAGCombines. Also: remove else after return and braces around single-line blocks to match the coding standards. --- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 17 +---------------- llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 6 ++---- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 6129c1a89912e..18333547535ac 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -2662,22 +2662,7 @@ SDValue DAGCombiner::visitPTRADD(SDNode *N) { if (isNullConstant(X) || (YIsConstant && ZOneUse) || (YIsConstant && N0OneUse) || (N0OneUse && ZOneUse && !ZIsConstant)) { SDValue Add = DAG.getNode(ISD::ADD, DL, IntVT, {Y, Z}); - - // Calling visit() can replace the Add node with ISD::DELETED_NODE if - // there aren't any users, so keep a handle around whilst we visit it. - HandleSDNode ADDHandle(Add); - - SDValue VisitedAdd = visit(Add.getNode()); - if (VisitedAdd) { - // If visit() returns the same node, it means the SDNode was RAUW'd, and - // therefore we have to load the new value to perform the checks whether - // the reassociation fold is profitable. - if (VisitedAdd.getNode() == Add.getNode()) - Add = ADDHandle.getValue(); - else - Add = VisitedAdd; - } - + AddToWorklist(Add.getNode()); return DAG.getMemBasePlusOffset(X, Add, DL, SDNodeFlags()); } diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index 1483a5f4d5b95..40695638cf942 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -8146,11 +8146,9 @@ SDValue SelectionDAG::getMemBasePlusOffset(SDValue Ptr, SDValue Offset, assert(Offset.getValueType().isInteger()); EVT BasePtrVT = Ptr.getValueType(); if (!this->getTarget().shouldPreservePtrArith( - this->getMachineFunction().getFunction())) { + this->getMachineFunction().getFunction())) return getNode(ISD::ADD, DL, BasePtrVT, Ptr, Offset, Flags); - } else { - return getNode(ISD::PTRADD, DL, BasePtrVT, Ptr, Offset, Flags); - } + return getNode(ISD::PTRADD, DL, BasePtrVT, Ptr, Offset, Flags); } /// Returns true if memcpy source is constant data. From f6a4cda119e7825a571b24feaa21223a86103d15 Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Thu, 15 May 2025 09:23:10 -0400 Subject: [PATCH 03/12] Remove DAGCombine comment referring to Morello and CHERI --- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 18333547535ac..ef281c733cc74 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -2623,8 +2623,6 @@ SDValue DAGCombiner::foldSubToAvg(SDNode *N, const SDLoc &DL) { /// Try to fold a pointer arithmetic node. /// This needs to be done separately from normal addition, because pointer /// addition is not commutative. -/// This function was adapted from DAGCombiner::visitPTRADD() from the Morello -/// project, which is based on CHERI. SDValue DAGCombiner::visitPTRADD(SDNode *N) { SDValue N0 = N->getOperand(0); SDValue N1 = N->getOperand(1); From d5f083ed1dadf2cbfa914c8f886059120f8a7ee0 Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Fri, 16 May 2025 03:56:44 -0400 Subject: [PATCH 04/12] Add the pointer's EVT to the signature of TM::shouldPreservePtrArith() --- llvm/include/llvm/Target/TargetMachine.h | 11 ++++++++--- llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/llvm/include/llvm/Target/TargetMachine.h b/llvm/include/llvm/Target/TargetMachine.h index 8536439f0a2d4..8ce01a84517dd 100644 --- a/llvm/include/llvm/Target/TargetMachine.h +++ b/llvm/include/llvm/Target/TargetMachine.h @@ -35,6 +35,7 @@ namespace llvm { class AAManager; using ModulePassManager = PassManager; +struct EVT; class Function; class GlobalValue; class MachineModuleInfoWrapperPass; @@ -469,9 +470,13 @@ class TargetMachine { } /// True if target has some particular form of dealing with pointer arithmetic - /// semantics. False if pointer arithmetic should not be preserved for passes - /// such as instruction selection, and can fallback to regular arithmetic. - virtual bool shouldPreservePtrArith(const Function &F) const { return false; } + /// semantics for pointers with the given value type. False if pointer + /// arithmetic should not be preserved for passes such as instruction + /// selection, and can fallback to regular arithmetic. + virtual bool shouldPreservePtrArith(const Function &F, + const EVT &PtrVT) const { + return false; + } /// Create a pass configuration object to be used by addPassToEmitX methods /// for generating a pipeline of CodeGen passes. diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index 40695638cf942..826e1f775f95c 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -8146,7 +8146,7 @@ SDValue SelectionDAG::getMemBasePlusOffset(SDValue Ptr, SDValue Offset, assert(Offset.getValueType().isInteger()); EVT BasePtrVT = Ptr.getValueType(); if (!this->getTarget().shouldPreservePtrArith( - this->getMachineFunction().getFunction())) + this->getMachineFunction().getFunction(), BasePtrVT)) return getNode(ISD::ADD, DL, BasePtrVT, Ptr, Offset, Flags); return getNode(ISD::PTRADD, DL, BasePtrVT, Ptr, Offset, Flags); } From 759f0656a9bc4e50bdc6add4d218b9425120d8cc Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Fri, 16 May 2025 05:22:02 -0400 Subject: [PATCH 05/12] Move shouldPreservePtrArith from TargetMachine to TargetLoweringBase, add a comment to state that it should be removed eventually, and reorder if/else cases to avoid a negated condition where shouldPreservePtrArith is called. --- llvm/include/llvm/CodeGen/TargetLowering.h | 9 +++++++++ llvm/include/llvm/Target/TargetMachine.h | 10 ---------- llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 8 ++++---- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h index 03099e9ad44dc..f9d3d82ab4a84 100644 --- a/llvm/include/llvm/CodeGen/TargetLowering.h +++ b/llvm/include/llvm/CodeGen/TargetLowering.h @@ -3483,6 +3483,15 @@ class TargetLoweringBase { /// doing arithmetic on boolean types virtual bool shouldExpandCmpUsingSelects(EVT VT) const { return false; } + /// True if target has some particular form of dealing with pointer arithmetic + /// semantics for pointers with the given value type. False if pointer + /// arithmetic should not be preserved for passes such as instruction + /// selection, and can fallback to regular arithmetic. + /// This should be removed when PTRADD nodes are widely supported by backends. + virtual bool shouldPreservePtrArith(const Function &F, EVT PtrVT) const { + return false; + } + /// Does this target support complex deinterleaving virtual bool isComplexDeinterleavingSupported() const { return false; } diff --git a/llvm/include/llvm/Target/TargetMachine.h b/llvm/include/llvm/Target/TargetMachine.h index 8ce01a84517dd..906926729ed74 100644 --- a/llvm/include/llvm/Target/TargetMachine.h +++ b/llvm/include/llvm/Target/TargetMachine.h @@ -35,7 +35,6 @@ namespace llvm { class AAManager; using ModulePassManager = PassManager; -struct EVT; class Function; class GlobalValue; class MachineModuleInfoWrapperPass; @@ -469,15 +468,6 @@ class TargetMachine { return false; } - /// True if target has some particular form of dealing with pointer arithmetic - /// semantics for pointers with the given value type. False if pointer - /// arithmetic should not be preserved for passes such as instruction - /// selection, and can fallback to regular arithmetic. - virtual bool shouldPreservePtrArith(const Function &F, - const EVT &PtrVT) const { - return false; - } - /// Create a pass configuration object to be used by addPassToEmitX methods /// for generating a pipeline of CodeGen passes. virtual TargetPassConfig *createPassConfig(PassManagerBase &PM) { diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index 826e1f775f95c..e31b88ab9ea46 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -8145,10 +8145,10 @@ SDValue SelectionDAG::getMemBasePlusOffset(SDValue Ptr, SDValue Offset, const SDNodeFlags Flags) { assert(Offset.getValueType().isInteger()); EVT BasePtrVT = Ptr.getValueType(); - if (!this->getTarget().shouldPreservePtrArith( - this->getMachineFunction().getFunction(), BasePtrVT)) - return getNode(ISD::ADD, DL, BasePtrVT, Ptr, Offset, Flags); - return getNode(ISD::PTRADD, DL, BasePtrVT, Ptr, Offset, Flags); + if (TLI->shouldPreservePtrArith(this->getMachineFunction().getFunction(), + BasePtrVT)) + return getNode(ISD::PTRADD, DL, BasePtrVT, Ptr, Offset, Flags); + return getNode(ISD::ADD, DL, BasePtrVT, Ptr, Offset, Flags); } /// Returns true if memcpy source is constant data. From 3a38633e7782afa0f3336cd0ee2f0614346e801a Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Mon, 19 May 2025 04:44:07 -0400 Subject: [PATCH 06/12] Add (YIsConstant && ZIsConstant) case to the DAGCombine. --- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index ef281c733cc74..9f5ad27222b97 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -2656,9 +2656,11 @@ SDValue DAGCombiner::visitPTRADD(SDNode *N) { // * x is a null pointer; or // * y is a constant and z has one use; or // * y is a constant and (ptradd x, y) has one use; or + // * y and z are both constants; or // * (ptradd x, y) and z have one use and z is not a constant. if (isNullConstant(X) || (YIsConstant && ZOneUse) || - (YIsConstant && N0OneUse) || (N0OneUse && ZOneUse && !ZIsConstant)) { + (YIsConstant && N0OneUse) || (YIsConstant && ZIsConstant) || + (N0OneUse && ZOneUse && !ZIsConstant)) { SDValue Add = DAG.getNode(ISD::ADD, DL, IntVT, {Y, Z}); AddToWorklist(Add.getNode()); return DAG.getMemBasePlusOffset(X, Add, DL, SDNodeFlags()); From 2ccad11fe32f3fbed04c0944d7119fda9c9387e8 Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Mon, 19 May 2025 08:46:23 -0400 Subject: [PATCH 07/12] Add ISD::isPtrAdd() and fix a typo. --- llvm/include/llvm/CodeGen/ISDOpcodes.h | 6 +++++- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 4 ++-- llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 3 +-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/CodeGen/ISDOpcodes.h b/llvm/include/llvm/CodeGen/ISDOpcodes.h index abae3e921117f..e3989ad62d237 100644 --- a/llvm/include/llvm/CodeGen/ISDOpcodes.h +++ b/llvm/include/llvm/CodeGen/ISDOpcodes.h @@ -1502,7 +1502,7 @@ enum NodeType { // Outputs: [rv], output chain, glue PATCHPOINT, - // PTRADD represents pointer arithmatic semantics, for targets that opt in + // PTRADD represents pointer arithmetic semantics, for targets that opt in // using shouldPreservePtrArith(). // ptr = PTRADD ptr, offset PTRADD, @@ -1734,6 +1734,10 @@ inline bool isExtVecInRegOpcode(unsigned Opcode) { Opcode == ISD::SIGN_EXTEND_VECTOR_INREG; } +inline bool isPtrAdd(unsigned Opcode) { + return (Opcode == ISD::ADD) || (Opcode == ISD::PTRADD); +} + namespace GlobalISel { /// Return the operation corresponding to !(X op Y), where 'op' is a valid /// SetCC operation. The U bit of the condition code has different meanings diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 9f5ad27222b97..28211deb87b23 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -1097,7 +1097,7 @@ bool DAGCombiner::reassociationCanBreakAddressingModePattern(unsigned Opc, // (load/store (add, (add, x, y), offset2)) -> // (load/store (add, (add, x, offset2), y)). - if (N0.getOpcode() != ISD::ADD && N0.getOpcode() != ISD::PTRADD) + if (!ISD::isPtrAdd(N0.getOpcode())) return false; // Check for vscale addressing modes. @@ -2391,7 +2391,7 @@ static bool canFoldInAddressingMode(SDNode *N, SDNode *Use, SelectionDAG &DAG, } TargetLowering::AddrMode AM; - if (N->getOpcode() == ISD::ADD || N->getOpcode() == ISD::PTRADD) { + if (ISD::isPtrAdd(N->getOpcode())) { AM.HasBaseReg = true; ConstantSDNode *Offset = dyn_cast(N->getOperand(1)); if (Offset) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index e31b88ab9ea46..1dce2e951b9bf 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -5641,8 +5641,7 @@ bool SelectionDAG::isADDLike(SDValue Op, bool NoWrap) const { bool SelectionDAG::isBaseWithConstantOffset(SDValue Op) const { return Op.getNumOperands() == 2 && isa(Op.getOperand(1)) && - (Op.getOpcode() == ISD::ADD || Op.getOpcode() == ISD::PTRADD || - isADDLike(Op)); + (ISD::isPtrAdd(Op.getOpcode()) || isADDLike(Op)); } bool SelectionDAG::isKnownNeverNaN(SDValue Op, bool SNaN, From 4c6ba494eccd1918050761904f3f9a7b9bc5a8dd Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Tue, 20 May 2025 03:19:53 -0400 Subject: [PATCH 08/12] Move isPtrAdd to SDNode/SDValue; rename it to isAnyAdd --- llvm/include/llvm/CodeGen/ISDOpcodes.h | 4 ---- llvm/include/llvm/CodeGen/SelectionDAGNodes.h | 10 ++++++++++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 4 ++-- llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/llvm/include/llvm/CodeGen/ISDOpcodes.h b/llvm/include/llvm/CodeGen/ISDOpcodes.h index e3989ad62d237..3c748b701c9ff 100644 --- a/llvm/include/llvm/CodeGen/ISDOpcodes.h +++ b/llvm/include/llvm/CodeGen/ISDOpcodes.h @@ -1734,10 +1734,6 @@ inline bool isExtVecInRegOpcode(unsigned Opcode) { Opcode == ISD::SIGN_EXTEND_VECTOR_INREG; } -inline bool isPtrAdd(unsigned Opcode) { - return (Opcode == ISD::ADD) || (Opcode == ISD::PTRADD); -} - namespace GlobalISel { /// Return the operation corresponding to !(X op Y), where 'op' is a valid /// SetCC operation. The U bit of the condition code has different meanings diff --git a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h index cfefceea8f0fe..65cc9152fbbe2 100644 --- a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h +++ b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h @@ -213,6 +213,7 @@ class SDValue { inline bool isTargetOpcode() const; inline bool isMachineOpcode() const; inline bool isUndef() const; + inline bool isAnyAdd() const; inline unsigned getMachineOpcode() const; inline const DebugLoc &getDebugLoc() const; inline void dump() const; @@ -697,6 +698,11 @@ END_TWO_BYTE_PACK() return NodeType == ISD::UNDEF || NodeType == ISD::POISON; } + /// Returns true if the node type is ADD or PTRADD. + bool isAnyAdd() const { + return NodeType == ISD::ADD || NodeType == ISD::PTRADD; + } + /// Test if this node is a memory intrinsic (with valid pointer information). bool isMemIntrinsic() const { return SDNodeBits.IsMemIntrinsic; } @@ -1270,6 +1276,10 @@ inline bool SDValue::isUndef() const { return Node->isUndef(); } +inline bool SDValue::isAnyAdd() const { + return Node->isAnyAdd(); +} + inline bool SDValue::use_empty() const { return !Node->hasAnyUseOfValue(ResNo); } diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 28211deb87b23..dfcd0d0694780 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -1097,7 +1097,7 @@ bool DAGCombiner::reassociationCanBreakAddressingModePattern(unsigned Opc, // (load/store (add, (add, x, y), offset2)) -> // (load/store (add, (add, x, offset2), y)). - if (!ISD::isPtrAdd(N0.getOpcode())) + if (!N0.isAnyAdd()) return false; // Check for vscale addressing modes. @@ -2391,7 +2391,7 @@ static bool canFoldInAddressingMode(SDNode *N, SDNode *Use, SelectionDAG &DAG, } TargetLowering::AddrMode AM; - if (ISD::isPtrAdd(N->getOpcode())) { + if (N->isAnyAdd()) { AM.HasBaseReg = true; ConstantSDNode *Offset = dyn_cast(N->getOperand(1)); if (Offset) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index 1dce2e951b9bf..72bd4f0284ec2 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -5641,7 +5641,7 @@ bool SelectionDAG::isADDLike(SDValue Op, bool NoWrap) const { bool SelectionDAG::isBaseWithConstantOffset(SDValue Op) const { return Op.getNumOperands() == 2 && isa(Op.getOperand(1)) && - (ISD::isPtrAdd(Op.getOpcode()) || isADDLike(Op)); + (Op.isAnyAdd() || isADDLike(Op)); } bool SelectionDAG::isKnownNeverNaN(SDValue Op, bool SNaN, From c0ed6a2f859fff09a39475af12e6321098c5a553 Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Tue, 20 May 2025 03:29:19 -0400 Subject: [PATCH 09/12] Fix formatting --- llvm/include/llvm/CodeGen/SelectionDAGNodes.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h index 65cc9152fbbe2..b7a527b81019d 100644 --- a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h +++ b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h @@ -1276,9 +1276,7 @@ inline bool SDValue::isUndef() const { return Node->isUndef(); } -inline bool SDValue::isAnyAdd() const { - return Node->isAnyAdd(); -} +inline bool SDValue::isAnyAdd() const { return Node->isAnyAdd(); } inline bool SDValue::use_empty() const { return !Node->hasAnyUseOfValue(ResNo); From 1239537577ae406457928109d8d0cabe78ad588f Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Wed, 21 May 2025 04:11:02 -0400 Subject: [PATCH 10/12] Add PTRADD handling to SDAG::getNode() That involves early folds and (technically overly restrictive) assertions on the operand types. --- llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index 72bd4f0284ec2..ffedec2975a7b 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -7341,10 +7341,18 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT, case ISD::OR: case ISD::XOR: case ISD::ADD: + case ISD::PTRADD: case ISD::SUB: assert(VT.isInteger() && "This operator does not apply to FP types!"); assert(N1.getValueType() == N2.getValueType() && N1.getValueType() == VT && "Binary operator types must match!"); + // The equal operand types requirement is unnecessarily strong for PTRADD. + // However, the SelectionDAGBuilder does not generate PTRADDs with different + // operand types, and we'd need to re-implement GEP's non-standard wrapping + // logic everywhere where PTRADDs may be folded or combined to properly + // support them. If/when we introduce pointer types to the SDAG, we will + // need to relax this constraint. + // (X ^|+- 0) -> X. This commonly occurs when legalizing i64 values, so // it's worth handling here. if (N2CV && N2CV->isZero()) @@ -7696,6 +7704,7 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT, std::swap(N1, N2); } else { switch (Opcode) { + case ISD::PTRADD: case ISD::SUB: // fold op(undef, arg2) -> undef, fold op(poison, arg2) ->poison. return N1.getOpcode() == ISD::POISON ? getPOISON(VT) : getUNDEF(VT); @@ -7723,6 +7732,7 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT, return getConstant(0, DL, VT); [[fallthrough]]; case ISD::ADD: + case ISD::PTRADD: case ISD::SUB: case ISD::UDIV: case ISD::SDIV: From 4edcf2f7ab30158d102cd109af638ecc435b21fe Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Thu, 22 May 2025 03:07:23 -0400 Subject: [PATCH 11/12] Split the DAGCombiner::visitPtrAdd() implementation off to put it into a separate PR. --- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 81 ------------------- 1 file changed, 81 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index dfcd0d0694780..350895208a4fb 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -414,7 +414,6 @@ namespace { SDValue visitADDLike(SDNode *N); SDValue visitADDLikeCommutative(SDValue N0, SDValue N1, SDNode *LocReference); - SDValue visitPTRADD(SDNode *N); SDValue visitSUB(SDNode *N); SDValue visitADDSAT(SDNode *N); SDValue visitSUBSAT(SDNode *N); @@ -1854,7 +1853,6 @@ SDValue DAGCombiner::visit(SDNode *N) { case ISD::TokenFactor: return visitTokenFactor(N); case ISD::MERGE_VALUES: return visitMERGE_VALUES(N); case ISD::ADD: return visitADD(N); - case ISD::PTRADD: return visitPTRADD(N); case ISD::SUB: return visitSUB(N); case ISD::SADDSAT: case ISD::UADDSAT: return visitADDSAT(N); @@ -2620,85 +2618,6 @@ SDValue DAGCombiner::foldSubToAvg(SDNode *N, const SDLoc &DL) { return SDValue(); } -/// Try to fold a pointer arithmetic node. -/// This needs to be done separately from normal addition, because pointer -/// addition is not commutative. -SDValue DAGCombiner::visitPTRADD(SDNode *N) { - SDValue N0 = N->getOperand(0); - SDValue N1 = N->getOperand(1); - EVT PtrVT = N0.getValueType(); - EVT IntVT = N1.getValueType(); - SDLoc DL(N); - - // fold (ptradd undef, y) -> undef - if (N0.isUndef()) - return N0; - - // fold (ptradd x, undef) -> undef - if (N1.isUndef()) - return DAG.getUNDEF(PtrVT); - - // fold (ptradd x, 0) -> x - if (isNullConstant(N1)) - return N0; - - if (N0.getOpcode() == ISD::PTRADD && - !reassociationCanBreakAddressingModePattern(ISD::PTRADD, DL, N, N0, N1)) { - SDValue X = N0.getOperand(0); - SDValue Y = N0.getOperand(1); - SDValue Z = N1; - bool N0OneUse = N0.hasOneUse(); - bool YIsConstant = DAG.isConstantIntBuildVectorOrConstantInt(Y); - bool ZIsConstant = DAG.isConstantIntBuildVectorOrConstantInt(Z); - bool ZOneUse = Z.hasOneUse(); - - // (ptradd (ptradd x, y), z) -> (ptradd x, (add y, z)) if: - // * x is a null pointer; or - // * y is a constant and z has one use; or - // * y is a constant and (ptradd x, y) has one use; or - // * y and z are both constants; or - // * (ptradd x, y) and z have one use and z is not a constant. - if (isNullConstant(X) || (YIsConstant && ZOneUse) || - (YIsConstant && N0OneUse) || (YIsConstant && ZIsConstant) || - (N0OneUse && ZOneUse && !ZIsConstant)) { - SDValue Add = DAG.getNode(ISD::ADD, DL, IntVT, {Y, Z}); - AddToWorklist(Add.getNode()); - return DAG.getMemBasePlusOffset(X, Add, DL, SDNodeFlags()); - } - - // TODO: There is another possible fold here that was proven useful. - // It would be this: - // - // (ptradd (ptradd x, y), z) -> (ptradd (ptradd x, z), y) if: - // * (ptradd x, y) has one use; and - // * y is a constant; and - // * z is not a constant. - // - // In some cases, specifically in AArch64's FEAT_CPA, it exposes the - // opportunity to select more complex instructions such as SUBPT and - // MSUBPT. However, a hypothetical corner case has been found that we could - // not avoid. Consider this (pseudo-POSIX C): - // - // char *foo(char *x, int z) {return (x + LARGE_CONSTANT) + z;} - // char *p = mmap(LARGE_CONSTANT); - // char *q = foo(p, -LARGE_CONSTANT); - // - // Then x + LARGE_CONSTANT is one-past-the-end, so valid, and a - // further + z takes it back to the start of the mapping, so valid, - // regardless of the address mmap gave back. However, if mmap gives you an - // address < LARGE_CONSTANT (ignoring high bits), x - LARGE_CONSTANT will - // borrow from the high bits (with the subsequent + z carrying back into - // the high bits to give you a well-defined pointer) and thus trip - // FEAT_CPA's pointer corruption checks. - // - // We leave this fold as an opportunity for future work, addressing the - // corner case for FEAT_CPA, as well as reconciling the solution with the - // more general application of pointer arithmetic in other future targets. - } - - return SDValue(); -} - /// Try to fold a 'not' shifted sign-bit with add/sub with constant operand into /// a shift and add with a different constant. static SDValue foldAddSubOfSignBit(SDNode *N, const SDLoc &DL, From 7329a37e54a40e6ffbb6aca3199fb1cb5a5aa6df Mon Sep 17 00:00:00 2001 From: Fabian Ritter Date: Mon, 26 May 2025 03:17:57 -0400 Subject: [PATCH 12/12] Remove redundant flag argument --- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index b3227d9b589dd..6459c0b9be30c 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -9138,8 +9138,7 @@ bool SelectionDAGBuilder::visitMemPCpyCall(const CallInst &I) { Size = DAG.getSExtOrTrunc(Size, sdl, Dst.getValueType()); // Adjust return pointer to point just past the last dst byte. - SDNodeFlags Flags; - SDValue DstPlusSize = DAG.getMemBasePlusOffset(Dst, Size, sdl, Flags); + SDValue DstPlusSize = DAG.getMemBasePlusOffset(Dst, Size, sdl); setValue(&I, DstPlusSize); return true; }