Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 5 additions & 0 deletions llvm/include/llvm/Target/TargetMachine.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
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
103 changes: 100 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,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,
Expand Down
10 changes: 8 additions & 2 deletions 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,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.
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
Loading