Skip to content

Commit 30bb17d

Browse files
committed
[AMDGPU][SDAG] Add ISD::PTRADD DAG combines
This patch focuses on generic DAG combines, plus an AMDGPU-target-specific one that is closely connected. The generic DAG combine is based on a part of PR #105669 by @rgwott, which was adapted from work by @jrtc27, @arichardson, @davidchisnall in the CHERI/Morello LLVM tree. I added some parts and removed several disjuncts from the reassociation condition: - `isNullConstant(X)`, since there are address spaces where 0 is a perfectly normal value that shouldn't be treated specially, - `(YIsConstant && ZOneUse)` and `(N0OneUse && ZOneUse && !ZIsConstant)`, since they cause regressions in AMDGPU. For SWDEV-516125.
1 parent 477db40 commit 30bb17d

File tree

4 files changed

+201
-135
lines changed

4 files changed

+201
-135
lines changed

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,7 @@ namespace {
419419
SDValue visitADDLike(SDNode *N);
420420
SDValue visitADDLikeCommutative(SDValue N0, SDValue N1,
421421
SDNode *LocReference);
422+
SDValue visitPTRADD(SDNode *N);
422423
SDValue visitSUB(SDNode *N);
423424
SDValue visitADDSAT(SDNode *N);
424425
SDValue visitSUBSAT(SDNode *N);
@@ -1138,7 +1139,7 @@ bool DAGCombiner::reassociationCanBreakAddressingModePattern(unsigned Opc,
11381139
return true;
11391140
}
11401141

1141-
if (Opc != ISD::ADD)
1142+
if (Opc != ISD::ADD && Opc != ISD::PTRADD)
11421143
return false;
11431144

11441145
auto *C2 = dyn_cast<ConstantSDNode>(N1);
@@ -1858,6 +1859,7 @@ SDValue DAGCombiner::visit(SDNode *N) {
18581859
case ISD::TokenFactor: return visitTokenFactor(N);
18591860
case ISD::MERGE_VALUES: return visitMERGE_VALUES(N);
18601861
case ISD::ADD: return visitADD(N);
1862+
case ISD::PTRADD: return visitPTRADD(N);
18611863
case ISD::SUB: return visitSUB(N);
18621864
case ISD::SADDSAT:
18631865
case ISD::UADDSAT: return visitADDSAT(N);
@@ -2627,6 +2629,93 @@ SDValue DAGCombiner::foldSubToAvg(SDNode *N, const SDLoc &DL) {
26272629
return SDValue();
26282630
}
26292631

2632+
/// Try to fold a pointer arithmetic node.
2633+
/// This needs to be done separately from normal addition, because pointer
2634+
/// addition is not commutative.
2635+
SDValue DAGCombiner::visitPTRADD(SDNode *N) {
2636+
SDValue N0 = N->getOperand(0);
2637+
SDValue N1 = N->getOperand(1);
2638+
EVT PtrVT = N0.getValueType();
2639+
EVT IntVT = N1.getValueType();
2640+
SDLoc DL(N);
2641+
2642+
// This is already ensured by an assert in SelectionDAG::getNode(). Several
2643+
// combines here depend on this assumption.
2644+
assert(PtrVT == IntVT &&
2645+
"PTRADD with different operand types is not supported");
2646+
2647+
// fold (ptradd undef, y) -> undef
2648+
if (N0.isUndef())
2649+
return N0;
2650+
2651+
// fold (ptradd x, undef) -> undef
2652+
if (N1.isUndef())
2653+
return DAG.getUNDEF(PtrVT);
2654+
2655+
// fold (ptradd x, 0) -> x
2656+
if (isNullConstant(N1))
2657+
return N0;
2658+
2659+
// fold (ptradd 0, x) -> x
2660+
if (isNullConstant(N0))
2661+
return N1;
2662+
2663+
if (N0.getOpcode() == ISD::PTRADD &&
2664+
!reassociationCanBreakAddressingModePattern(ISD::PTRADD, DL, N, N0, N1)) {
2665+
SDValue X = N0.getOperand(0);
2666+
SDValue Y = N0.getOperand(1);
2667+
SDValue Z = N1;
2668+
bool N0OneUse = N0.hasOneUse();
2669+
bool YIsConstant = DAG.isConstantIntBuildVectorOrConstantInt(Y);
2670+
bool ZIsConstant = DAG.isConstantIntBuildVectorOrConstantInt(Z);
2671+
2672+
// (ptradd (ptradd x, y), z) -> (ptradd x, (add y, z)) if:
2673+
// * y is a constant and (ptradd x, y) has one use; or
2674+
// * y and z are both constants.
2675+
if ((YIsConstant && N0OneUse) || (YIsConstant && ZIsConstant)) {
2676+
SDNodeFlags Flags;
2677+
// If both additions in the original were NUW, the new ones are as well.
2678+
if (N->getFlags().hasNoUnsignedWrap() &&
2679+
N0->getFlags().hasNoUnsignedWrap())
2680+
Flags |= SDNodeFlags::NoUnsignedWrap;
2681+
SDValue Add = DAG.getNode(ISD::ADD, DL, IntVT, {Y, Z}, Flags);
2682+
AddToWorklist(Add.getNode());
2683+
return DAG.getMemBasePlusOffset(X, Add, DL, Flags);
2684+
}
2685+
2686+
// TODO: There is another possible fold here that was proven useful.
2687+
// It would be this:
2688+
//
2689+
// (ptradd (ptradd x, y), z) -> (ptradd (ptradd x, z), y) if:
2690+
// * (ptradd x, y) has one use; and
2691+
// * y is a constant; and
2692+
// * z is not a constant.
2693+
//
2694+
// In some cases, specifically in AArch64's FEAT_CPA, it exposes the
2695+
// opportunity to select more complex instructions such as SUBPT and
2696+
// MSUBPT. However, a hypothetical corner case has been found that we could
2697+
// not avoid. Consider this (pseudo-POSIX C):
2698+
//
2699+
// char *foo(char *x, int z) {return (x + LARGE_CONSTANT) + z;}
2700+
// char *p = mmap(LARGE_CONSTANT);
2701+
// char *q = foo(p, -LARGE_CONSTANT);
2702+
//
2703+
// Then x + LARGE_CONSTANT is one-past-the-end, so valid, and a
2704+
// further + z takes it back to the start of the mapping, so valid,
2705+
// regardless of the address mmap gave back. However, if mmap gives you an
2706+
// address < LARGE_CONSTANT (ignoring high bits), x - LARGE_CONSTANT will
2707+
// borrow from the high bits (with the subsequent + z carrying back into
2708+
// the high bits to give you a well-defined pointer) and thus trip
2709+
// FEAT_CPA's pointer corruption checks.
2710+
//
2711+
// We leave this fold as an opportunity for future work, addressing the
2712+
// corner case for FEAT_CPA, as well as reconciling the solution with the
2713+
// more general application of pointer arithmetic in other future targets.
2714+
}
2715+
2716+
return SDValue();
2717+
}
2718+
26302719
/// Try to fold a 'not' shifted sign-bit with add/sub with constant operand into
26312720
/// a shift and add with a different constant.
26322721
static SDValue foldAddSubOfSignBit(SDNode *N, const SDLoc &DL,
@@ -14955,6 +15044,7 @@ SDValue DAGCombiner::visitAssertAlign(SDNode *N) {
1495515044
default:
1495615045
break;
1495715046
case ISD::ADD:
15047+
case ISD::PTRADD:
1495815048
case ISD::SUB: {
1495915049
unsigned AlignShift = Log2(AL);
1496015050
SDValue LHS = N0.getOperand(0);

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,7 @@ SITargetLowering::SITargetLowering(const TargetMachine &TM,
938938
setOperationAction(ISD::FP_ROUND, MVT::v2f16, Custom);
939939

940940
setTargetDAGCombine({ISD::ADD,
941+
ISD::PTRADD,
941942
ISD::UADDO_CARRY,
942943
ISD::SUB,
943944
ISD::USUBO_CARRY,
@@ -14935,6 +14936,52 @@ SDValue SITargetLowering::performAddCombine(SDNode *N,
1493514936
return SDValue();
1493614937
}
1493714938

14939+
SDValue SITargetLowering::performPtrAddCombine(SDNode *N,
14940+
DAGCombinerInfo &DCI) const {
14941+
SelectionDAG &DAG = DCI.DAG;
14942+
EVT VT = N->getValueType(0);
14943+
SDLoc DL(N);
14944+
SDValue N0 = N->getOperand(0);
14945+
SDValue N1 = N->getOperand(1);
14946+
14947+
if (N1.getOpcode() == ISD::ADD) {
14948+
// (ptradd x, (add y, z)) -> (ptradd (ptradd x, y), z) if z is a constant,
14949+
// y is not, and (add y, z) is used only once.
14950+
// (ptradd x, (add y, z)) -> (ptradd (ptradd x, z), y) if y is a constant,
14951+
// z is not, and (add y, z) is used only once.
14952+
// The goal is to move constant offsets to the outermost ptradd, to create
14953+
// more opportunities to fold offsets into memory instructions.
14954+
// Together with the generic combines in DAGCombiner.cpp, this also
14955+
// implements (ptradd (ptradd x, y), z) -> (ptradd (ptradd x, z), y)).
14956+
//
14957+
// This transform is here instead of in the general DAGCombiner as it can
14958+
// turn in-bounds pointer arithmetic out-of-bounds, which is problematic for
14959+
// AArch64's CPA.
14960+
SDValue X = N0;
14961+
SDValue Y = N1.getOperand(0);
14962+
SDValue Z = N1.getOperand(1);
14963+
bool N1OneUse = N1.hasOneUse();
14964+
bool YIsConstant = DAG.isConstantIntBuildVectorOrConstantInt(Y);
14965+
bool ZIsConstant = DAG.isConstantIntBuildVectorOrConstantInt(Z);
14966+
if ((ZIsConstant != YIsConstant) && N1OneUse) {
14967+
SDNodeFlags Flags;
14968+
// If both additions in the original were NUW, the new ones are as well.
14969+
if (N->getFlags().hasNoUnsignedWrap() &&
14970+
N1->getFlags().hasNoUnsignedWrap())
14971+
Flags |= SDNodeFlags::NoUnsignedWrap;
14972+
14973+
if (YIsConstant)
14974+
std::swap(Y, Z);
14975+
14976+
SDValue Inner = DAG.getMemBasePlusOffset(X, Y, DL, Flags);
14977+
DCI.AddToWorklist(Inner.getNode());
14978+
return DAG.getMemBasePlusOffset(Inner, Z, DL, Flags);
14979+
}
14980+
}
14981+
14982+
return SDValue();
14983+
}
14984+
1493814985
SDValue SITargetLowering::performSubCombine(SDNode *N,
1493914986
DAGCombinerInfo &DCI) const {
1494014987
SelectionDAG &DAG = DCI.DAG;
@@ -15467,6 +15514,8 @@ SDValue SITargetLowering::PerformDAGCombine(SDNode *N,
1546715514
switch (N->getOpcode()) {
1546815515
case ISD::ADD:
1546915516
return performAddCombine(N, DCI);
15517+
case ISD::PTRADD:
15518+
return performPtrAddCombine(N, DCI);
1547015519
case ISD::SUB:
1547115520
return performSubCombine(N, DCI);
1547215521
case ISD::UADDO_CARRY:

llvm/lib/Target/AMDGPU/SIISelLowering.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ class SITargetLowering final : public AMDGPUTargetLowering {
218218
DAGCombinerInfo &DCI) const;
219219

220220
SDValue performAddCombine(SDNode *N, DAGCombinerInfo &DCI) const;
221+
SDValue performPtrAddCombine(SDNode *N, DAGCombinerInfo &DCI) const;
221222
SDValue performAddCarrySubCarryCombine(SDNode *N, DAGCombinerInfo &DCI) const;
222223
SDValue performSubCombine(SDNode *N, DAGCombinerInfo &DCI) const;
223224
SDValue performFAddCombine(SDNode *N, DAGCombinerInfo &DCI) const;

0 commit comments

Comments
 (0)