Skip to content

Commit c7aea91

Browse files
ritter-x2aritter-x2a
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 41ee0e7 commit c7aea91

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);
@@ -2628,6 +2630,93 @@ SDValue DAGCombiner::foldSubToAvg(SDNode *N, const SDLoc &DL) {
26282630
return SDValue();
26292631
}
26302632

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

943943
setTargetDAGCombine({ISD::ADD,
944+
ISD::PTRADD,
944945
ISD::UADDO_CARRY,
945946
ISD::SUB,
946947
ISD::USUBO_CARRY,
@@ -14944,6 +14945,52 @@ SDValue SITargetLowering::performAddCombine(SDNode *N,
1494414945
return SDValue();
1494514946
}
1494614947

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

‎llvm/lib/Target/AMDGPU/SIISelLowering.h‎

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

221221
SDValue performAddCombine(SDNode *N, DAGCombinerInfo &DCI) const;
222+
SDValue performPtrAddCombine(SDNode *N, DAGCombinerInfo &DCI) const;
222223
SDValue performAddCarrySubCarryCombine(SDNode *N, DAGCombinerInfo &DCI) const;
223224
SDValue performSubCombine(SDNode *N, DAGCombinerInfo &DCI) const;
224225
SDValue performFAddCombine(SDNode *N, DAGCombinerInfo &DCI) const;

0 commit comments

Comments
 (0)