Skip to content

Commit 07292b7

Browse files
authored
[LIR][SCEVExpander] Restore original flags when aborting transform (#82362)
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 0c13a89 commit 07292b7

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
}
@@ -491,6 +509,8 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
491509

492510
void rememberInstruction(Value *I);
493511

512+
void rememberFlags(Instruction *I);
513+
494514
bool isNormalAddRecExprPHI(PHINode *PN, Instruction *IncV, const Loop *L);
495515

496516
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->hasNoSignedWrap();
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->setHasNoSignedWrap(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)) {
@@ -1481,6 +1513,7 @@ Value *SCEVExpander::expand(const SCEV *S) {
14811513
V = fixupLCSSAFormFor(V);
14821514
} else {
14831515
for (Instruction *I : DropPoisonGeneratingInsts) {
1516+
rememberFlags(I);
14841517
I->dropPoisonGeneratingFlagsAndMetadata();
14851518
// See if we can re-infer from first principles any of the flags we just
14861519
// dropped.
@@ -1521,6 +1554,11 @@ void SCEVExpander::rememberInstruction(Value *I) {
15211554
DoInsert(I);
15221555
}
15231556

1557+
void SCEVExpander::rememberFlags(Instruction *I) {
1558+
// If we already have flags for the instruction, keep the existing ones.
1559+
OrigFlags.try_emplace(I, PoisonFlags(I));
1560+
}
1561+
15241562
void SCEVExpander::replaceCongruentIVInc(
15251563
PHINode *&Phi, PHINode *&OrigPhi, Loop *L, const DominatorTree *DT,
15261564
SmallVectorImpl<WeakTrackingVH> &DeadInsts) {
@@ -2318,6 +2356,10 @@ void SCEVExpanderCleaner::cleanup() {
23182356
if (ResultUsed)
23192357
return;
23202358

2359+
// Restore original poison flags.
2360+
for (auto [I, Flags] : Expander.OrigFlags)
2361+
Flags.apply(I);
2362+
23212363
auto InsertedInstructions = Expander.getAllInsertedInstructions();
23222364
#ifndef NDEBUG
23232365
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)