Skip to content

Commit ac18c80

Browse files
committed
[LIR][SCEVExpander] Restore original flags when aborting transform
SCEVExpanderCleaner will currently remove instructions created by SCEVExpander, but not restore poison generating flags that it may have dropped. As such, running LIR can currently spuriously drop flags without performing any transforms. Fix this by keeping track of original instruction flags in SCEVExpander. Fixes #82337.
1 parent fcd6549 commit ac18c80

File tree

3 files changed

+65
-3
lines changed

3 files changed

+65
-3
lines changed

llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,26 @@ struct SCEVOperand {
4141
const SCEV* S;
4242
};
4343

44+
struct PoisonFlags {
45+
unsigned NUW : 1;
46+
unsigned NSW : 1;
47+
unsigned Exact : 1;
48+
unsigned Disjoint : 1;
49+
unsigned NNeg : 1;
50+
51+
PoisonFlags(const Instruction *I);
52+
void apply(Instruction *I);
53+
};
54+
4455
/// This class uses information about analyze scalars to rewrite expressions
4556
/// in canonical form.
4657
///
4758
/// Clients should create an instance of this class when rewriting is needed,
4859
/// and destroy it when finished to allow the release of the associated
4960
/// memory.
5061
class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
62+
friend class SCEVExpanderCleaner;
63+
5164
ScalarEvolution &SE;
5265
const DataLayout &DL;
5366

@@ -70,6 +83,10 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
7083
/// InsertedValues/InsertedPostIncValues.
7184
SmallPtrSet<Value *, 16> ReusedValues;
7285

86+
/// Original flags of instructions for which they were modified. Used
87+
/// by SCEVExpanderCleaner to undo changes.
88+
DenseMap<AssertingVH<Instruction>, PoisonFlags> OrigFlags;
89+
7390
// The induction variables generated.
7491
SmallVector<WeakVH, 2> InsertedIVs;
7592

@@ -188,6 +205,7 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
188205
InsertedValues.clear();
189206
InsertedPostIncValues.clear();
190207
ReusedValues.clear();
208+
OrigFlags.clear();
191209
ChainedPhis.clear();
192210
InsertedIVs.clear();
193211
}
@@ -483,6 +501,8 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
483501

484502
void rememberInstruction(Value *I);
485503

504+
void rememberFlags(Instruction *I);
505+
486506
bool isNormalAddRecExprPHI(PHINode *PN, Instruction *IncV, const Loop *L);
487507

488508
bool isExpandedAddRecExprPHI(PHINode *PN, Instruction *IncV, const Loop *L);

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,37 @@ cl::opt<unsigned> llvm::SCEVCheapExpansionBudget(
4343

4444
using namespace PatternMatch;
4545

46+
PoisonFlags::PoisonFlags(const Instruction *I) {
47+
NUW = false;
48+
NSW = false;
49+
Exact = false;
50+
Disjoint = false;
51+
NNeg = false;
52+
if (auto *OBO = dyn_cast<OverflowingBinaryOperator>(I)) {
53+
NUW = OBO->hasNoUnsignedWrap();
54+
NSW = OBO->hasNoUnsignedWrap();
55+
}
56+
if (auto *PEO = dyn_cast<PossiblyExactOperator>(I))
57+
Exact = PEO->isExact();
58+
if (auto *PDI = dyn_cast<PossiblyDisjointInst>(I))
59+
Disjoint = PDI->isDisjoint();
60+
if (auto *PNI = dyn_cast<PossiblyNonNegInst>(I))
61+
NNeg = PNI->hasNonNeg();
62+
}
63+
64+
void PoisonFlags::apply(Instruction *I) {
65+
if (isa<OverflowingBinaryOperator>(I)) {
66+
I->setHasNoUnsignedWrap(NUW);
67+
I->setHasNoUnsignedWrap(NSW);
68+
}
69+
if (isa<PossiblyExactOperator>(I))
70+
I->setIsExact(Exact);
71+
if (auto *PDI = dyn_cast<PossiblyDisjointInst>(I))
72+
PDI->setIsDisjoint(Disjoint);
73+
if (auto *PNI = dyn_cast<PossiblyNonNegInst>(I))
74+
PNI->setNonNeg(NNeg);
75+
}
76+
4677
/// ReuseOrCreateCast - Arrange for there to be a cast of V to Ty at IP,
4778
/// reusing an existing cast if a suitable one (= dominating IP) exists, or
4879
/// creating a new one.
@@ -724,6 +755,7 @@ bool SCEVExpander::hoistIVInc(Instruction *IncV, Instruction *InsertPos,
724755
auto FixupPoisonFlags = [this](Instruction *I) {
725756
// Drop flags that are potentially inferred from old context and infer flags
726757
// in new context.
758+
rememberFlags(I);
727759
I->dropPoisonGeneratingFlags();
728760
if (auto *OBO = dyn_cast<OverflowingBinaryOperator>(I))
729761
if (auto Flags = SE.getStrengthenedNoWrapFlagsFromBinOp(OBO)) {
@@ -1472,6 +1504,7 @@ Value *SCEVExpander::expand(const SCEV *S) {
14721504
V = fixupLCSSAFormFor(V);
14731505
} else {
14741506
for (Instruction *I : DropPoisonGeneratingInsts) {
1507+
rememberFlags(I);
14751508
I->dropPoisonGeneratingFlagsAndMetadata();
14761509
// See if we can re-infer from first principles any of the flags we just
14771510
// dropped.
@@ -1512,6 +1545,11 @@ void SCEVExpander::rememberInstruction(Value *I) {
15121545
DoInsert(I);
15131546
}
15141547

1548+
void SCEVExpander::rememberFlags(Instruction *I) {
1549+
// If we already have flags for the instruction, keep the existing ones.
1550+
OrigFlags.try_emplace(I, PoisonFlags(I));
1551+
}
1552+
15151553
void SCEVExpander::replaceCongruentIVInc(
15161554
PHINode *&Phi, PHINode *&OrigPhi, Loop *L, const DominatorTree *DT,
15171555
SmallVectorImpl<WeakTrackingVH> &DeadInsts) {
@@ -2309,6 +2347,10 @@ void SCEVExpanderCleaner::cleanup() {
23092347
if (ResultUsed)
23102348
return;
23112349

2350+
// Restore original poison flags.
2351+
for (auto [I, Flags] : Expander.OrigFlags)
2352+
Flags.apply(I);
2353+
23122354
auto InsertedInstructions = Expander.getAllInsertedInstructions();
23132355
#ifndef NDEBUG
23142356
SmallPtrSet<Instruction *, 8> InsertedSet(InsertedInstructions.begin(),

llvm/test/Transforms/LoopIdiom/pr82337.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
22
; RUN: opt -S -passes=loop-idiom < %s | FileCheck %s
33

4-
; FIXME: The poison flags should be preserved, as no transform takes place.
4+
; The poison flags should be preserved, as no transform takes place.
55
define void @test(ptr %p.end, ptr %p.start) {
66
; CHECK-LABEL: define void @test(
77
; CHECK-SAME: ptr [[P_END:%.*]], ptr [[P_START:%.*]]) {
88
; CHECK-NEXT: entry:
99
; CHECK-NEXT: [[P_END_INT:%.*]] = ptrtoint ptr [[P_END]] to i64
1010
; CHECK-NEXT: [[P_START_INT:%.*]] = ptrtoint ptr [[P_START]] to i64
11-
; CHECK-NEXT: [[DIST:%.*]] = sub i64 [[P_END_INT]], [[P_START_INT]]
12-
; CHECK-NEXT: [[LEN:%.*]] = lshr i64 [[DIST]], 5
11+
; CHECK-NEXT: [[DIST:%.*]] = sub nuw i64 [[P_END_INT]], [[P_START_INT]]
12+
; CHECK-NEXT: [[LEN:%.*]] = lshr exact i64 [[DIST]], 5
1313
; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[P_END]], [[P_START]]
1414
; CHECK-NEXT: br i1 [[CMP]], label [[EXIT:%.*]], label [[PREHEADER:%.*]]
1515
; CHECK: preheader:

0 commit comments

Comments
 (0)