Skip to content

Commit 27bf604

Browse files
krystian-andrzejewskiigcbot
authored andcommitted
Extend support for creating SCEVs iteratively
This change is to avoid stack overflow issues related to creating scalar evolution expressions. The previous solution can lead to unintendent scenarios when building an expression chain requires to use a recursive approach extensively. Moreover, there is an improvement in the quality of these expressions because of creating a final expression of the phi instruction only in the recursive function.
1 parent 7840e13 commit 27bf604

File tree

2 files changed

+146
-129
lines changed

2 files changed

+146
-129
lines changed
Lines changed: 146 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,97 +1,172 @@
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;
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+
+}
2889
+
29-
/// Mark SCEVUnknown Phis currently being processed by getRangeRef.
30-
SmallPtrSet<const PHINode *, 6> PendingPhiRanges;
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+
}
31117

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 @@
118+
return nullptr;
119+
@@ -7004,6 +7024,7 @@ const SCEV *ScalarEvolution::createSCEVIter(Value *V) {
36120

37121
Stack.emplace_back(V, true);
38122
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-
+ }
45123
+
46124
while (!Stack.empty()) {
47125
auto E = Stack.pop_back_val();
48126
Value *CurV = E.getPointer();
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) {
127+
@@ -7023,14 +7044,15 @@ const SCEV *ScalarEvolution::createSCEVIter(Value *V) {
128+
CreatedSCEV = getOperandsToCreate(CurV, Ops);
129+
}
130+
131+
- if (CreatedSCEV) {
132+
+ if (CreatedSCEV)
63133
insertValueToMap(CurV, CreatedSCEV);
64-
+ if (isa<PHINode>(CurV))
65-
+ PendingPhiSCEVIter.erase(cast<PHINode>(CurV));
66-
} else {
134+
- } else {
135+
+ else {
67136
// Queue CurV for SCEV creation, followed by its's operands which need to
68137
// be constructed first.
69138
Stack.emplace_back(CurV, true);
70139
- for (Value *Op : Ops)
71-
+ if (isa<PHINode>(CurV))
72-
+ PendingPhiSCEVIter.insert(cast<PHINode>(CurV));
73140
+ for (Value *Op : Ops) {
74141
Stack.emplace_back(Op, false);
75142
+ }
76143
}
77144
}
78145

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

82-
case Instruction::PHI:
150+
- case Instruction::PHI:
83151
- // Keep constructing SCEVs' for phis recursively for now.
84-
+ for (auto& Op : cast<PHINode>(U)->operands())
85-
+ Ops.push_back(Op);
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)
86168
return nullptr;
169+
+ }
87170

88171
case Instruction::Select: {
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);
172+
// Check if U is a select that can be simplified to a SCEVUnknown.

external/llvm/releases/14.0.0/patches_external/0003-ScalarEvolutionStackOverflowWindows.patch

Lines changed: 0 additions & 58 deletions
This file was deleted.

0 commit comments

Comments
 (0)