Skip to content

Commit 1a815dd

Browse files
jrtc27resistor
authored andcommitted
[CodeGen] Don't create combine loops for PTRADD with opaque constants
We were implicitly assuming here that, if one of Y or Z is a constant, either the other is not or we will have already (in the nested PTRADD case) constant-folded both of them into one, and thus don't need to consider the possibility. However, opaque constants exist and do not get constant-folded, so we can end up cycling between PTRADD+ADD and PTRADD+PTRADD. Catch this case by checking the other is not a constant and skipping all but one combine if so. No test is provided since I've yet to be able to trigger it in CHERI LLVM, only downstream in Codasip LLVM and Morello LLVM (and only when targeting CHERI-RISC-V even in Morello LLVM), both of which enable SCEV and thus do interesting loop transformations. Fixes: 05fd67a ("[CodeGen][CHERI-Generic] Support commuting capability offsets when safe")
1 parent f17670e commit 1a815dd

File tree

1 file changed

+22
-14
lines changed

1 file changed

+22
-14
lines changed

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,8 +2789,9 @@ SDValue DAGCombiner::visitPTRADD(SDNode *N) {
27892789
// * x is a null pointer; or
27902790
// * the add can be constant-folded; or
27912791
// * the add can be combined and z is not a constant; or
2792-
// * y is a constant and z has one use; or
2793-
// * y is a constant and (ptradd x, y) has one use; or
2792+
// * y is a constant and z has one use and z is not a constant; or
2793+
// * y is a constant and (ptradd x, y) has one use and z is not a
2794+
// constant; or
27942795
// * (ptradd x, y) and z have one use and z is not a constant.
27952796
//
27962797
// Some of these overly-restrictive conditions are to not obfuscate CAndAddr
@@ -2799,12 +2800,15 @@ SDValue DAGCombiner::visitPTRADD(SDNode *N) {
27992800
// and canonicalise it to a PTRMASK.
28002801
//
28012802
// Commute: (ptradd (ptradd x, y), z) -> (ptradd (ptradd x, z), y) if:
2802-
// * y and z have the same sign and y is a constant.
2803+
// * y and z have the same sign and only y is a constant.
28032804
//
28042805
// This allows immediate addressing modes to be used. Note that we need to be
28052806
// careful to ensure we don't transiently become unrepresentable if the
28062807
// original DAG does not already do so, and this is the case if both PTRADDs
28072808
// have the same sign.
2809+
//
2810+
// Note we must be careful to handle the case that both are
2811+
// constants, as opaque constants will not be constant-folded.
28082812
if (N0.getOpcode() == ISD::PTRADD &&
28092813
!reassociationCanBreakAddressingModePattern(ISD::PTRADD, DL, N, N0, N1)) {
28102814
SDValue X = N0.getOperand(0);
@@ -2827,34 +2831,38 @@ SDValue DAGCombiner::visitPTRADD(SDNode *N) {
28272831
}
28282832
LLVM_DEBUG(dbgs() << "visitPTRADD() add operand:"; Add.dump(&DAG));
28292833
assert(Add->getOpcode() != ISD::DELETED_NODE && "Deleted Node used");
2830-
if (isNullConstant(X) ||
2831-
DAG.isConstantIntBuildVectorOrConstantInt(Add) ||
2832-
(VisitedAdd && !DAG.isConstantIntBuildVectorOrConstantInt(Z)) ||
2833-
(DAG.isConstantIntBuildVectorOrConstantInt(Y) && Z.hasOneUse()) ||
2834-
(DAG.isConstantIntBuildVectorOrConstantInt(Y) && N0.hasOneUse()) ||
2835-
(N0.hasOneUse() && Z.hasOneUse() &&
2836-
!DAG.isConstantIntBuildVectorOrConstantInt(Z)))
2834+
bool IsZConst = DAG.isConstantIntBuildVectorOrConstantInt(Z);
2835+
if (isNullConstant(X) || DAG.isConstantIntBuildVectorOrConstantInt(Add) ||
2836+
(VisitedAdd && !IsZConst) ||
2837+
(DAG.isConstantIntBuildVectorOrConstantInt(Y) && Z.hasOneUse() &&
2838+
!IsZConst) ||
2839+
(DAG.isConstantIntBuildVectorOrConstantInt(Y) && N0.hasOneUse() &&
2840+
!IsZConst) ||
2841+
(N0.hasOneUse() && Z.hasOneUse() && !IsZConst))
28372842
return DAG.getMemBasePlusOffset(X, Add, DL);
2838-
if (DAG.SignBitIsSame(Y, Z) && DAG.isConstantIntBuildVectorOrConstantInt(Y))
2843+
if (DAG.SignBitIsSame(Y, Z) &&
2844+
DAG.isConstantIntBuildVectorOrConstantInt(Y) &&
2845+
!DAG.isConstantIntBuildVectorOrConstantInt(Z))
28392846
return GetPTRADD2(X, Z, Y);
28402847
}
28412848

28422849
// Transform: (ptradd x, (add y, z)) -> (ptradd (ptradd x, y), z) if:
28432850
// * both y and z have the same sign and z is a constant.
28442851
//
28452852
// Transform: (ptradd x, (add y, z)) -> (ptradd (ptradd x, z), y) if:
2846-
// * both y and z have the same sign and y is a constant.
2853+
// * both y and z have the same sign and y is a constant (and
2854+
// implicitly not z due to the previous transform).
28472855
//
28482856
// As above, this allows for immediate addressing modes.
28492857
if (N1.getOpcode() == ISD::ADD) {
28502858
SDValue X = N0;
28512859
SDValue Y = N1.getOperand(0);
28522860
SDValue Z = N1.getOperand(1);
28532861
if (DAG.SignBitIsSame(Y, Z)) {
2854-
if (DAG.isConstantIntBuildVectorOrConstantInt(Y))
2855-
return GetPTRADD2(X, Z, Y);
28562862
if (DAG.isConstantIntBuildVectorOrConstantInt(Z))
28572863
return GetPTRADD2(X, Y, Z);
2864+
if (DAG.isConstantIntBuildVectorOrConstantInt(Y))
2865+
return GetPTRADD2(X, Z, Y);
28582866
}
28592867
}
28602868

0 commit comments

Comments
 (0)