Skip to content

Commit 116dc70

Browse files
author
Chris Jackson
committed
[DebugInfo][LSR] Add more stringent checks on IV selection and salvage
attempts Prevent the selection of IVs that have a SCEV containing an undef. Also prevent salvaging attempts for values for which a SCEV could not be created by ScalarEvolution and have only SCEVUknown. Reviewed by: Orlando Differential Revision: https://reviews.llvm.org/D111810
1 parent 2ead347 commit 116dc70

File tree

3 files changed

+36
-23
lines changed

3 files changed

+36
-23
lines changed

llvm/include/llvm/Analysis/ScalarEvolution.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,9 @@ class ScalarEvolution {
537537
/// Notify this ScalarEvolution that \p User directly uses SCEVs in \p Ops.
538538
void registerUser(const SCEV *User, ArrayRef<const SCEV *> Ops);
539539

540+
/// Return true if the SCEV expression contains an undef value.
541+
bool containsUndefs(const SCEV *S) const;
542+
540543
/// Return a SCEV expression for the full generality of the specified
541544
/// expression.
542545
const SCEV *getSCEV(Value *V);

llvm/lib/Analysis/ScalarEvolution.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12375,7 +12375,7 @@ SCEVAddRecExpr::getPostIncExpr(ScalarEvolution &SE) const {
1237512375
}
1237612376

1237712377
// Return true when S contains at least an undef value.
12378-
static inline bool containsUndefs(const SCEV *S) {
12378+
bool ScalarEvolution::containsUndefs(const SCEV *S) const {
1237912379
return SCEVExprContains(S, [](const SCEV *S) {
1238012380
if (const auto *SU = dyn_cast<SCEVUnknown>(S))
1238112381
return isa<UndefValue>(SU->getValue());

llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6236,7 +6236,8 @@ DbgRewriteSalvageableDVIs(llvm::Loop *L, ScalarEvolution &SE,
62366236
}
62376237

62386238
/// Identify and cache salvageable DVI locations and expressions along with the
6239-
/// corresponding SCEV(s). Also ensure that the DVI is not deleted before
6239+
/// corresponding SCEV(s). Also ensure that the DVI is not deleted between
6240+
/// cacheing and salvaging.
62406241
static void
62416242
DbgGatherSalvagableDVI(Loop *L, ScalarEvolution &SE,
62426243
SmallVector<DVIRecoveryRec, 2> &SalvageableDVISCEVs,
@@ -6257,6 +6258,16 @@ DbgGatherSalvagableDVI(Loop *L, ScalarEvolution &SE,
62576258
!SE.isSCEVable(DVI->getVariableLocationOp(0)->getType()))
62586259
continue;
62596260

6261+
// SCEVUnknown wraps an llvm::Value, it does not have a start and stride.
6262+
// Therefore no translation to DIExpression is performed.
6263+
const SCEV *S = SE.getSCEV(DVI->getVariableLocationOp(0));
6264+
if (isa<SCEVUnknown>(S))
6265+
continue;
6266+
6267+
// Avoid wasting resources generating an expression containing undef.
6268+
if (SE.containsUndefs(S))
6269+
continue;
6270+
62606271
SalvageableDVISCEVs.push_back(
62616272
{DVI, DVI->getExpression(), DVI->getRawLocation(),
62626273
SE.getSCEV(DVI->getVariableLocationOp(0))});
@@ -6270,33 +6281,32 @@ DbgGatherSalvagableDVI(Loop *L, ScalarEvolution &SE,
62706281
/// surviving subsequent transforms.
62716282
static llvm::PHINode *GetInductionVariable(const Loop &L, ScalarEvolution &SE,
62726283
const LSRInstance &LSR) {
6273-
// For now, just pick the first IV generated and inserted. Ideally pick an IV
6274-
// that is unlikely to be optimised away by subsequent transforms.
6284+
6285+
auto IsSuitableIV = [&](PHINode *P) {
6286+
if (!SE.isSCEVable(P->getType()))
6287+
return false;
6288+
if (const SCEVAddRecExpr *Rec = dyn_cast<SCEVAddRecExpr>(SE.getSCEV(P)))
6289+
return Rec->isAffine() && !SE.containsUndefs(SE.getSCEV(P));
6290+
return false;
6291+
};
6292+
6293+
// For now, just pick the first IV that was generated and inserted by
6294+
// ScalarEvolution. Ideally pick an IV that is unlikely to be optimised away
6295+
// by subsequent transforms.
62756296
for (const WeakVH &IV : LSR.getScalarEvolutionIVs()) {
62766297
if (!IV)
62776298
continue;
62786299

6279-
assert(isa<PHINode>(&*IV) && "Expected PhI node.");
6280-
if (SE.isSCEVable((*IV).getType())) {
6281-
PHINode *Phi = dyn_cast<PHINode>(&*IV);
6282-
LLVM_DEBUG(dbgs() << "scev-salvage: IV : " << *IV
6283-
<< "with SCEV: " << *SE.getSCEV(Phi) << "\n");
6284-
return Phi;
6285-
}
6286-
}
6300+
// There should only be PHI node IVs.
6301+
PHINode *P = cast<PHINode>(&*IV);
62876302

6288-
for (PHINode &Phi : L.getHeader()->phis()) {
6289-
if (!SE.isSCEVable(Phi.getType()))
6290-
continue;
6291-
6292-
const llvm::SCEV *PhiSCEV = SE.getSCEV(&Phi);
6293-
if (const llvm::SCEVAddRecExpr *Rec = dyn_cast<SCEVAddRecExpr>(PhiSCEV))
6294-
if (!Rec->isAffine())
6295-
continue;
6303+
if (IsSuitableIV(P))
6304+
return P;
6305+
}
62966306

6297-
LLVM_DEBUG(dbgs() << "scev-salvage: Selected IV from loop header: " << Phi
6298-
<< " with SCEV: " << *PhiSCEV << "\n");
6299-
return &Phi;
6307+
for (PHINode &P : L.getHeader()->phis()) {
6308+
if (IsSuitableIV(&P))
6309+
return &P;
63006310
}
63016311
return nullptr;
63026312
}

0 commit comments

Comments
 (0)