Skip to content

[SelectionDAG] Introduce ISD::PTRADD #140017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 28, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions llvm/include/llvm/CodeGen/ISDOpcodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
9 changes: 9 additions & 0 deletions llvm/include/llvm/CodeGen/TargetLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/Target/TargetSelectionDAG.td
Original file line number Diff line number Diff line change
Expand Up @@ -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]>;
Expand Down
86 changes: 83 additions & 3 deletions llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<ConstantSDNode>(N->getOperand(1));
if (Offset)
Expand Down Expand Up @@ -2617,6 +2620,83 @@ 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
// * (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});
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,
Expand Down
6 changes: 5 additions & 1 deletion llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5641,7 +5641,8 @@ bool SelectionDAG::isADDLike(SDValue Op, bool NoWrap) const {

bool SelectionDAG::isBaseWithConstantOffset(SDValue Op) const {
return Op.getNumOperands() == 2 && isa<ConstantSDNode>(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,
Expand Down Expand Up @@ -8144,6 +8145,9 @@ SDValue SelectionDAG::getMemBasePlusOffset(SDValue Ptr, SDValue Offset,
const SDNodeFlags Flags) {
assert(Offset.getValueType().isInteger());
EVT BasePtrVT = Ptr.getValueType();
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);
}

Expand Down
19 changes: 9 additions & 10 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the Flags argument of getMemBasePlusOffset have a default value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I've now removed the unnecessary explicit argument setting here, thanks! I think we could actually use NUW here since this operation computes the position one-past-the-end of a memcpy'd memory range (which should either be within or one-past-the-end of an allocated object, which may not wrap according to the IR LangRef), but let's not add more functionality changes to this PR.

SDValue DstPlusSize = DAG.getMemBasePlusOffset(Dst, Size, sdl, Flags);
setValue(&I, DstPlusSize);
return true;
}
Expand Down Expand Up @@ -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(),
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down