Skip to content

Commit 58d171d

Browse files
krystian-andrzejewskiigcbot
authored andcommitted
Revert "Extend support for creating SCEVs iteratively"
This is a simple revert.
1 parent 0a24db2 commit 58d171d

File tree

2 files changed

+129
-146
lines changed

2 files changed

+129
-146
lines changed
Lines changed: 71 additions & 146 deletions
Original file line numberDiff line numberDiff line change
@@ -1,172 +1,97 @@
1-
diff --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h
2-
index 51558a37b..a2069980a 100644
3-
--- a/llvm/include/llvm/Analysis/ScalarEvolution.h
4-
+++ b/llvm/include/llvm/Analysis/ScalarEvolution.h
5-
@@ -1613,6 +1613,9 @@ private:
6-
/// Provide the special handling we need to analyze PHI SCEVs.
7-
const SCEV *createNodeForPHI(PHINode *PN);
8-
9-
+ /// Helper function to get a single start and back edge value from a phi node.
10-
+ std::pair<Value*, Value*> getStartAndBackEdgeValues(PHINode *PN);
11-
+
12-
/// Helper function called from createNodeForPHI.
13-
const SCEV *createAddRecFromPHI(PHINode *PN);
14-
15-
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
16-
index fc9517ebb..facfe1c7d 100644
17-
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
18-
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
19-
@@ -5529,10 +5529,11 @@ const SCEV *ScalarEvolution::createSimpleAffineAddRec(PHINode *PN,
20-
return PHISCEV;
21-
}
22-
23-
-const SCEV *ScalarEvolution::createAddRecFromPHI(PHINode *PN) {
24-
+std::pair<Value *, Value *>
25-
+ScalarEvolution::getStartAndBackEdgeValues(PHINode *PN) {
26-
const Loop *L = LI.getLoopFor(PN->getParent());
27-
if (!L || L->getHeader() != PN->getParent())
28-
- return nullptr;
29-
+ return {};
30-
31-
// The loop may have multiple entrances or multiple exits; we can analyze
32-
// this phi as an addrec if it has a unique entry value and a unique
33-
@@ -5555,10 +5556,18 @@ const SCEV *ScalarEvolution::createAddRecFromPHI(PHINode *PN) {
34-
}
35-
}
36-
if (!BEValueV || !StartValueV)
37-
- return nullptr;
38-
+ return {};
39-
+ return {StartValueV, BEValueV};
40-
+}
41-
42-
- assert(ValueExprMap.find_as(PN) == ValueExprMap.end() &&
43-
- "PHI node already processed?");
44-
+const SCEV *ScalarEvolution::createAddRecFromPHI(PHINode *PN) {
45-
+ // The loop may have multiple entrances or multiple exits; we can analyze
46-
+ // this phi as an addrec if it has a unique entry value and a unique
47-
+ // backedge value.
48-
+ std::pair<Value *, Value *> StartAndBEValueV = getStartAndBackEdgeValues(PN);
49-
+ Value *BEValueV = StartAndBEValueV.second, *StartValueV = StartAndBEValueV.first;
50-
+ if (!BEValueV || !StartValueV)
51-
+ return nullptr;
52-
53-
// First, try to find AddRec expression without creating a fictituos symbolic
54-
// value for PN.
55-
@@ -5572,6 +5581,7 @@ const SCEV *ScalarEvolution::createAddRecFromPHI(PHINode *PN) {
56-
// Using this symbolic name for the PHI, analyze the value coming around
57-
// the back-edge.
58-
const SCEV *BEValue = getSCEV(BEValueV);
59-
+ const Loop *L = LI.getLoopFor(PN->getParent());
60-
61-
// NOTE: If BEValue is loop invariant, we know that the PHI node just
62-
// has a special value for the first iteration of the loop.
63-
@@ -5772,8 +5782,15 @@ static bool IsAvailableOnEntry(const Loop *L, DominatorTree &DT, const SCEV *S,
64-
// Try to match a control flow sequence that branches out at BI and merges back
65-
// at Merge into a "C ? LHS : RHS" select pattern. Return true on a successful
66-
// match.
67-
-static bool BrPHIToSelect(DominatorTree &DT, BranchInst *BI, PHINode *Merge,
68-
- Value *&C, Value *&LHS, Value *&RHS) {
69-
+static bool BrPHIToSelect(DominatorTree &DT, PHINode *Merge, Value *&C,
70-
+ Value *&LHS, Value *&RHS) {
71-
+
72-
+ BasicBlock *IDom = DT[Merge->getParent()]->getIDom()->getBlock();
73-
+ assert(IDom && "At least the entry block should dominate PN");
74-
+
75-
+ auto *BI = dyn_cast<BranchInst>(IDom->getTerminator());
76-
+ if (!BI || !BI->isConditional())
77-
+ return false;
78-
C = BI->getCondition();
79-
80-
BasicBlockEdge LeftEdge(BI->getParent(), BI->getSuccessor(0));
81-
@@ -5802,6 +5819,11 @@ static bool BrPHIToSelect(DominatorTree &DT, BranchInst *BI, PHINode *Merge,
82-
return false;
83-
}
84-
85-
+static bool BrPHIToSelect(DominatorTree& DT, PHINode* Merge) {
86-
+ Value *Cond = nullptr, *LHS = nullptr, *RHS = nullptr;
87-
+ return BrPHIToSelect(DT, Merge, Cond, LHS, RHS);
88-
+}
1+
/*========================== begin_copyright_notice ============================
2+
3+
Copyright (C) 2023 Intel Corporation
4+
5+
SPDX-License-Identifier: MIT
6+
7+
============================= end_copyright_notice ===========================*/
8+
9+
/*========================== begin_copyright_notice ============================
10+
11+
Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
12+
See https://llvm.org/LICENSE.txt for license information.
13+
SPDX-License-Identifier: Apache-2.0 with LLVM-exception
14+
15+
============================= end_copyright_notice ===========================*/
16+
17+
# TODO: Once upstreamed, update with LLORG revision & adjust per community review
18+
19+
diff -Naur "a/llvm/include/llvm/Analysis/ScalarEvolution.h" "b/llvm/include/llvm/Analysis/ScalarEvolution.h"
20+
--- "a/llvm/include/llvm/Analysis/ScalarEvolution.h" 2023-09-27 19:27:08.174510600 +0200
21+
+++ "b/llvm/include/llvm/Analysis/ScalarEvolution.h" 2023-09-27 18:41:11.301171800 +0200
22+
@@ -1292,6 +1292,9 @@
23+
/// Mark predicate values currently being processed by isImpliedCond.
24+
SmallPtrSet<const Value *, 6> PendingLoopPredicates;
25+
26+
+ /// Set of Phis processed in createSCEVIter.
27+
+ SmallPtrSet<const PHINode*, 6> PendingPhiSCEVIter;
8928
+
90-
const SCEV *ScalarEvolution::createNodeFromSelectLikePHI(PHINode *PN) {
91-
auto IsReachable =
92-
[&](BasicBlock *BB) { return DT.isReachableFromEntry(BB); };
93-
@@ -5824,18 +5846,16 @@ const SCEV *ScalarEvolution::createNodeFromSelectLikePHI(PHINode *PN) {
94-
// V = phi [ %x, %left ], [ %y, %right ]
95-
//
96-
// as "select %cond, %x, %y"
97-
-
98-
- BasicBlock *IDom = DT[PN->getParent()]->getIDom()->getBlock();
99-
- assert(IDom && "At least the entry block should dominate PN");
100-
-
101-
- auto *BI = dyn_cast<BranchInst>(IDom->getTerminator());
102-
Value *Cond = nullptr, *LHS = nullptr, *RHS = nullptr;
103-
104-
- if (BI && BI->isConditional() &&
105-
- BrPHIToSelect(DT, BI, PN, Cond, LHS, RHS) &&
106-
+ // do not run over cycled PHIs while processing createNodeFromSelectLikePHI
107-
+ const SCEV* S = nullptr;
108-
+ if (BrPHIToSelect(DT, PN, Cond, LHS, RHS) &&
109-
IsAvailableOnEntry(L, DT, getSCEV(LHS), PN->getParent()) &&
110-
- IsAvailableOnEntry(L, DT, getSCEV(RHS), PN->getParent()))
111-
- return createNodeForSelectOrPHI(PN, Cond, LHS, RHS);
112-
+ IsAvailableOnEntry(L, DT, getSCEV(RHS), PN->getParent())) {
113-
+ S = createNodeForSelectOrPHI(PN, Cond, LHS, RHS);
114-
+ }
115-
+ return S;
116-
}
29+
/// Mark SCEVUnknown Phis currently being processed by getRangeRef.
30+
SmallPtrSet<const PHINode *, 6> PendingPhiRanges;
11731

118-
return nullptr;
119-
@@ -7004,6 +7024,7 @@ const SCEV *ScalarEvolution::createSCEVIter(Value *V) {
32+
diff -Naur "a/llvm/lib/Analysis/ScalarEvolution.cpp" "b/llvm/lib/Analysis/ScalarEvolution.cpp"
33+
--- "a/llvm/lib/Analysis/ScalarEvolution.cpp" 2023-09-27 19:27:08.189098000 +0200
34+
+++ "b/llvm/lib/Analysis/ScalarEvolution.cpp" 2023-09-27 19:05:40.601713600 +0200
35+
@@ -7004,6 +7004,13 @@
12036

12137
Stack.emplace_back(V, true);
12238
Stack.emplace_back(V, false);
39+
+
40+
+ if (isa<PHINode>(V) &&
41+
+ !PendingPhiSCEVIter.insert(cast<PHINode>(V)).second) {
42+
+ // cycled back to createSCEVIter while processing this Phi, bail
43+
+ return getUnknown(V);
44+
+ }
12345
+
12446
while (!Stack.empty()) {
12547
auto E = Stack.pop_back_val();
12648
Value *CurV = E.getPointer();
127-
@@ -7023,14 +7044,15 @@ const SCEV *ScalarEvolution::createSCEVIter(Value *V) {
128-
CreatedSCEV = getOperandsToCreate(CurV, Ops);
129-
}
130-
131-
- if (CreatedSCEV) {
132-
+ if (CreatedSCEV)
49+
@@ -7016,6 +7023,10 @@
50+
// If all operands have been visited already, create the SCEV.
51+
if (E.getInt()) {
52+
CreatedSCEV = createSCEV(CurV);
53+
+ } else if (isa<PHINode>(CurV) &&
54+
+ PendingPhiSCEVIter.count(cast<PHINode>(CurV)) > 0) {
55+
+ // cycled Phi, try to get its SCEV
56+
+ CreatedSCEV = createSCEV(CurV);
57+
} else {
58+
// Otherwise get the operands we need to create SCEV's for before creating
59+
// the SCEV for CurV. If the SCEV for CurV can be constructed trivially,
60+
@@ -7025,12 +7036,17 @@
61+
62+
if (CreatedSCEV) {
13363
insertValueToMap(CurV, CreatedSCEV);
134-
- } else {
135-
+ else {
64+
+ if (isa<PHINode>(CurV))
65+
+ PendingPhiSCEVIter.erase(cast<PHINode>(CurV));
66+
} else {
13667
// Queue CurV for SCEV creation, followed by its's operands which need to
13768
// be constructed first.
13869
Stack.emplace_back(CurV, true);
13970
- for (Value *Op : Ops)
71+
+ if (isa<PHINode>(CurV))
72+
+ PendingPhiSCEVIter.insert(cast<PHINode>(CurV));
14073
+ for (Value *Op : Ops) {
14174
Stack.emplace_back(Op, false);
14275
+ }
14376
}
14477
}
14578

146-
@@ -7152,9 +7174,24 @@ ScalarEvolution::getOperandsToCreate(Value *V, SmallVectorImpl<Value *> &Ops) {
147-
case Instruction::IntToPtr:
79+
@@ -7153,7 +7169,8 @@
14880
return getUnknown(V);
14981

150-
- case Instruction::PHI:
82+
case Instruction::PHI:
15183
- // Keep constructing SCEVs' for phis recursively for now.
152-
+ case Instruction::PHI: {
153-
+ PHINode *PN = cast<PHINode>(U);
154-
+
155-
+ if (PN->getNumIncomingValues() == 2) {
156-
+ std::pair<Value *, Value *> StartAndBEValueV =
157-
+ getStartAndBackEdgeValues(PN);
158-
+ if (StartAndBEValueV.first && StartAndBEValueV.second)
159-
+ // resolve recursively - may create cycles
160-
+ return nullptr;
161-
+ else if (BrPHIToSelect(DT, PN))
162-
+ // resolve iteratively - does not create cycles
163-
+ for (auto &Op : PN->operands())
164-
+ Ops.push_back(Op);
165-
+ }
166-
+
167-
+ // resolve iteratively - does not create SCEV cycles (trivially resolved)
84+
+ for (auto& Op : cast<PHINode>(U)->operands())
85+
+ Ops.push_back(Op);
16886
return nullptr;
169-
+ }
17087

17188
case Instruction::Select: {
172-
// Check if U is a select that can be simplified to a SCEVUnknown.
89+
@@ -13405,6 +13422,8 @@
90+
auto ValueIt = ValueExprMap.find_as(ValueAndOffset.first);
91+
if (ValueIt != ValueExprMap.end())
92+
ValueExprMap.erase(ValueIt);
93+
+ if (isa<PHINode>(ValueAndOffset.first))
94+
+ PendingPhiSCEVIter.erase(cast<PHINode>(ValueAndOffset.first));
95+
}
96+
}
97+
ExprValueMap.erase(ExprIt);
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*========================== begin_copyright_notice ============================
2+
3+
Copyright (C) 2023 Intel Corporation
4+
5+
SPDX-License-Identifier: MIT
6+
7+
============================= end_copyright_notice ===========================*/
8+
9+
/*========================== begin_copyright_notice ============================
10+
11+
Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
12+
See https://llvm.org/LICENSE.txt for license information.
13+
SPDX-License-Identifier: Apache-2.0 with LLVM-exception
14+
15+
============================= end_copyright_notice ===========================*/
16+
17+
# TODO: Once upstreamed, update with LLORG revision & adjust per community review
18+
19+
diff -Naur "a/llvm/include/llvm/Analysis/ScalarEvolution.h" "b/llvm/include/llvm/Analysis/ScalarEvolution.h"
20+
--- "a/llvm/include/llvm/Analysis/ScalarEvolution.h" 2023-12-19 12:51:55.043965800 +0100
21+
+++ "b/llvm/include/llvm/Analysis/ScalarEvolution.h" 2023-12-19 12:28:39.120139600 +0100
22+
@@ -1295,6 +1295,9 @@
23+
/// Set of Phis processed in createSCEVIter.
24+
SmallPtrSet<const PHINode*, 6> PendingPhiSCEVIter;
25+
26+
+ /// Set of Phis processed in createPhiNodeFromSelectLikePHI.
27+
+ SmallPtrSet<const PHINode*, 6> PendingPhiNodeFromSelectLikePHI;
28+
+
29+
/// Mark SCEVUnknown Phis currently being processed by getRangeRef.
30+
SmallPtrSet<const PHINode *, 6> PendingPhiRanges;
31+
32+
diff -Naur "a/llvm/lib/Analysis/ScalarEvolution.cpp" "b/llvm/lib/Analysis/ScalarEvolution.cpp"
33+
--- "a/llvm/lib/Analysis/ScalarEvolution.cpp" 2023-12-19 12:52:55.207074600 +0100
34+
+++ "b/llvm/lib/Analysis/ScalarEvolution.cpp" 2023-12-19 12:37:39.188525900 +0100
35+
@@ -5836,11 +5836,18 @@
36+
auto *BI = dyn_cast<BranchInst>(IDom->getTerminator());
37+
Value *Cond = nullptr, *LHS = nullptr, *RHS = nullptr;
38+
39+
- if (BI && BI->isConditional() &&
40+
- BrPHIToSelect(DT, BI, PN, Cond, LHS, RHS) &&
41+
- IsAvailableOnEntry(L, DT, getSCEV(LHS), PN->getParent()) &&
42+
- IsAvailableOnEntry(L, DT, getSCEV(RHS), PN->getParent()))
43+
- return createNodeForSelectOrPHI(PN, Cond, LHS, RHS);
44+
+ // do not run over cycled PHIs while processing createNodeFromSelectLikePHI
45+
+ if (PendingPhiNodeFromSelectLikePHI.insert(PN).second) {
46+
+ const SCEV* S = nullptr;
47+
+ if (BI && BI->isConditional() &&
48+
+ BrPHIToSelect(DT, BI, PN, Cond, LHS, RHS) &&
49+
+ IsAvailableOnEntry(L, DT, getSCEV(LHS), PN->getParent()) &&
50+
+ IsAvailableOnEntry(L, DT, getSCEV(RHS), PN->getParent())) {
51+
+ S = createNodeForSelectOrPHI(PN, Cond, LHS, RHS);
52+
+ }
53+
+ PendingPhiNodeFromSelectLikePHI.erase(PN);
54+
+ return S;
55+
+ }
56+
}
57+
58+
return nullptr;

0 commit comments

Comments
 (0)