Skip to content

Commit f2a5cd0

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 3e1e368 commit f2a5cd0

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
@@ -421,6 +421,7 @@ namespace {
421421
SDValue visitADDLike(SDNode *N);
422422
SDValue visitADDLikeCommutative(SDValue N0, SDValue N1,
423423
SDNode *LocReference);
424+
SDValue visitPTRADD(SDNode *N);
424425
SDValue visitSUB(SDNode *N);
425426
SDValue visitADDSAT(SDNode *N);
426427
SDValue visitSUBSAT(SDNode *N);
@@ -1140,7 +1141,7 @@ bool DAGCombiner::reassociationCanBreakAddressingModePattern(unsigned Opc,
11401141
return true;
11411142
}
11421143

1143-
if (Opc != ISD::ADD)
1144+
if (Opc != ISD::ADD && Opc != ISD::PTRADD)
11441145
return false;
11451146

11461147
auto *C2 = dyn_cast<ConstantSDNode>(N1);
@@ -1860,6 +1861,7 @@ SDValue DAGCombiner::visit(SDNode *N) {
18601861
case ISD::TokenFactor: return visitTokenFactor(N);
18611862
case ISD::MERGE_VALUES: return visitMERGE_VALUES(N);
18621863
case ISD::ADD: return visitADD(N);
1864+
case ISD::PTRADD: return visitPTRADD(N);
18631865
case ISD::SUB: return visitSUB(N);
18641866
case ISD::SADDSAT:
18651867
case ISD::UADDSAT: return visitADDSAT(N);
@@ -2630,6 +2632,93 @@ SDValue DAGCombiner::foldSubToAvg(SDNode *N, const SDLoc &DL) {
26302632
return SDValue();
26312633
}
26322634

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

947947
setTargetDAGCombine({ISD::ADD,
948+
ISD::PTRADD,
948949
ISD::UADDO_CARRY,
949950
ISD::SUB,
950951
ISD::USUBO_CARRY,
@@ -15095,6 +15096,52 @@ SDValue SITargetLowering::performAddCombine(SDNode *N,
1509515096
return SDValue();
1509615097
}
1509715098

15099+
SDValue SITargetLowering::performPtrAddCombine(SDNode *N,
15100+
DAGCombinerInfo &DCI) const {
15101+
SelectionDAG &DAG = DCI.DAG;
15102+
EVT VT = N->getValueType(0);
15103+
SDLoc DL(N);
15104+
SDValue N0 = N->getOperand(0);
15105+
SDValue N1 = N->getOperand(1);
15106+
15107+
if (N1.getOpcode() == ISD::ADD) {
15108+
// (ptradd x, (add y, z)) -> (ptradd (ptradd x, y), z) if z is a constant,
15109+
// y is not, and (add y, z) is used only once.
15110+
// (ptradd x, (add y, z)) -> (ptradd (ptradd x, z), y) if y is a constant,
15111+
// z is not, and (add y, z) is used only once.
15112+
// The goal is to move constant offsets to the outermost ptradd, to create
15113+
// more opportunities to fold offsets into memory instructions.
15114+
// Together with the generic combines in DAGCombiner.cpp, this also
15115+
// implements (ptradd (ptradd x, y), z) -> (ptradd (ptradd x, z), y)).
15116+
//
15117+
// This transform is here instead of in the general DAGCombiner as it can
15118+
// turn in-bounds pointer arithmetic out-of-bounds, which is problematic for
15119+
// AArch64's CPA.
15120+
SDValue X = N0;
15121+
SDValue Y = N1.getOperand(0);
15122+
SDValue Z = N1.getOperand(1);
15123+
bool N1OneUse = N1.hasOneUse();
15124+
bool YIsConstant = DAG.isConstantIntBuildVectorOrConstantInt(Y);
15125+
bool ZIsConstant = DAG.isConstantIntBuildVectorOrConstantInt(Z);
15126+
if ((ZIsConstant != YIsConstant) && N1OneUse) {
15127+
SDNodeFlags Flags;
15128+
// If both additions in the original were NUW, the new ones are as well.
15129+
if (N->getFlags().hasNoUnsignedWrap() &&
15130+
N1->getFlags().hasNoUnsignedWrap())
15131+
Flags |= SDNodeFlags::NoUnsignedWrap;
15132+
15133+
if (YIsConstant)
15134+
std::swap(Y, Z);
15135+
15136+
SDValue Inner = DAG.getMemBasePlusOffset(X, Y, DL, Flags);
15137+
DCI.AddToWorklist(Inner.getNode());
15138+
return DAG.getMemBasePlusOffset(Inner, Z, DL, Flags);
15139+
}
15140+
}
15141+
15142+
return SDValue();
15143+
}
15144+
1509815145
SDValue SITargetLowering::performSubCombine(SDNode *N,
1509915146
DAGCombinerInfo &DCI) const {
1510015147
SelectionDAG &DAG = DCI.DAG;
@@ -15633,6 +15680,8 @@ SDValue SITargetLowering::PerformDAGCombine(SDNode *N,
1563315680
switch (N->getOpcode()) {
1563415681
case ISD::ADD:
1563515682
return performAddCombine(N, DCI);
15683+
case ISD::PTRADD:
15684+
return performPtrAddCombine(N, DCI);
1563615685
case ISD::SUB:
1563715686
return performSubCombine(N, DCI);
1563815687
case ISD::UADDO_CARRY:

llvm/lib/Target/AMDGPU/SIISelLowering.h

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

222222
SDValue performAddCombine(SDNode *N, DAGCombinerInfo &DCI) const;
223+
SDValue performPtrAddCombine(SDNode *N, DAGCombinerInfo &DCI) const;
223224
SDValue performAddCarrySubCarryCombine(SDNode *N, DAGCombinerInfo &DCI) const;
224225
SDValue performSubCombine(SDNode *N, DAGCombinerInfo &DCI) const;
225226
SDValue performFAddCombine(SDNode *N, DAGCombinerInfo &DCI) const;

0 commit comments

Comments
 (0)