-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[LoopPeel] Add new option to peeling loops to convert PHI into IV #121104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
41fe38a
88272e4
ee26737
cd3fc9c
9b03954
c6f1a3a
0f41b26
b05a507
bfddeff
1b18665
27a46a1
df8f554
cd9b436
9d88973
39e8105
af6ab08
550256d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |||||
| #include "llvm/ADT/DenseMap.h" | ||||||
| #include "llvm/ADT/SmallVector.h" | ||||||
| #include "llvm/ADT/Statistic.h" | ||||||
| #include "llvm/Analysis/IVDescriptors.h" | ||||||
| #include "llvm/Analysis/Loads.h" | ||||||
| #include "llvm/Analysis/LoopInfo.h" | ||||||
| #include "llvm/Analysis/LoopIterator.h" | ||||||
|
|
@@ -151,6 +152,32 @@ namespace { | |||||
| // corresponding calls to g are determined and the code for computing | ||||||
| // x, y, and a can be removed. | ||||||
| // | ||||||
| // Similarly, there are cases where peeling makes Phi nodes loop-inductions | ||||||
| // (i.e., the value is increased or decreased by a fixed amount on every | ||||||
| // iteration). For example, consider the following function. | ||||||
| // | ||||||
| // #define N 100 | ||||||
| // void f(int a[], int b[]) { | ||||||
| // int im = N - 1; | ||||||
| // for (int i = 0; i < N; i++) { | ||||||
| // a[i] = b[i] + b[im]; | ||||||
| // im = i; | ||||||
| // } | ||||||
| // } | ||||||
| // | ||||||
| // The IR of the loop will look something like the following. | ||||||
| // | ||||||
| // %i = phi i32 [ 0, %entry ], [ %i.next, %for.body ] | ||||||
| // %im = phi i32 [ 99, %entry ], [ %i, %for.body ] | ||||||
| // ... | ||||||
| // %i.next = add nuw nsw i32 %i, 1 | ||||||
| // ... | ||||||
| // | ||||||
| // In this case, %im becomes a loop-induction variable by peeling 1 iteration, | ||||||
| // because %i is a loop-induction one. The peeling count can be determined by | ||||||
| // the same algorithm with loop-invariant case. Such peeling is profitable for | ||||||
| // loop-vectorization. | ||||||
| // | ||||||
| // The PhiAnalyzer class calculates how many times a loop should be | ||||||
| // peeled based on the above analysis of the phi nodes in the loop while | ||||||
| // respecting the maximum specified. | ||||||
|
|
@@ -177,11 +204,15 @@ class PhiAnalyzer { | |||||
| // becomes an invariant. | ||||||
| PeelCounter calculate(const Value &); | ||||||
|
|
||||||
| // Returns true if the \p Phi is an induction in the target loop. This | ||||||
| // function is a wrapper of `InductionDescriptor::isInductionPHI`. | ||||||
kasuga-fj marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| bool isInductionPHI(const PHINode *Phi) const; | ||||||
|
|
||||||
| const Loop &L; | ||||||
| const unsigned MaxIterations; | ||||||
|
|
||||||
| // Map of Values to number of iterations to invariance | ||||||
| SmallDenseMap<const Value *, PeelCounter> IterationsToInvariance; | ||||||
| // Map of Values to number of iterations to invariance or induction | ||||||
| SmallDenseMap<const Value *, PeelCounter> IterationsToInvarianceOrInduction; | ||||||
| }; | ||||||
|
|
||||||
| PhiAnalyzer::PhiAnalyzer(const Loop &L, unsigned MaxIterations) | ||||||
|
|
@@ -190,6 +221,67 @@ PhiAnalyzer::PhiAnalyzer(const Loop &L, unsigned MaxIterations) | |||||
| assert(MaxIterations > 0 && "no peeling is allowed?"); | ||||||
| } | ||||||
|
|
||||||
| // Test if \p Phi is induction variable or not. It can be checked by using SCEV, | ||||||
| // but it's expensive to calculate it here. Instead, we perform the cheaper | ||||||
| // checks, which cannot detect complex one but enough for some cases. | ||||||
kasuga-fj marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| bool PhiAnalyzer::isInductionPHI(const PHINode *Phi) const { | ||||||
| // Currently, we only support loops that consist of one basic block. In this | ||||||
| // case, the phi can become an IV if it has an incoming value from the basic | ||||||
| // block that this phi is also included. | ||||||
|
||||||
| // block that this phi is also included. | |
| // block where the phi is defined. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I don't really get why we need this limitation. The extension to multiple blocks seems trivial -- just need to look for the input from getLoopLatch() instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And once you do that you can also replace your loop with getIncomingValueForBlock.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose of that is to limit the number of incoming edges from the loop to one, and your suggestion looks better to me. Thanks! (Also, your point about the English language is also very much appreciated.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use if (!Visited.insert(Cur)) to avoid duplicate hash lookup.
kasuga-fj marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for this check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added them just in case because I don't fully understand how other similar functions (like isInductionPHI) handle these flags. This is not to say that I've found any cases where something weird happens without this check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I don't think this check is necessary. However, removing them happened undesirable peelings. It seems that this check just happens to work well to deflect undesired loops.
Uh oh!
There was an error while loading. Please reload this page.