[LV][NFC] Refactor structures used to maintain uncountable exit info #123219
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I've removed the HasUncountableEarlyExit variable, since we can already determine whether or not a loop has an early exit by seeing how many uncountable exit blocks we found. I have also deleted the old UncountableExitingBlocks and UncountableExitBlocks lists and replaced them with a single list of edges. This means we don't need to worry about keeping the list entries in sync and makes it clear which exiting block corresponds to which exit block.
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-vectorizers Author: David Sherwood (david-arm) ChangesI've removed the HasUncountableEarlyExit variable, since we can I have also deleted the old UncountableExitingBlocks and Full diff: https://github.com/llvm/llvm-project/pull/123219.diff 2 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h b/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
index 72fda911962ad2..5764e3a487f3e3 100644
--- a/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
+++ b/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
@@ -391,25 +391,24 @@ class LoopVectorizationLegality {
/// Returns true if the loop has an uncountable early exit, i.e. an
/// uncountable exit that isn't the latch block.
- bool hasUncountableEarlyExit() const { return HasUncountableEarlyExit; }
+ bool hasUncountableEarlyExit() const { return getUncountableEdges().size(); }
/// Returns the uncountable early exiting block.
BasicBlock *getUncountableEarlyExitingBlock() const {
- if (!HasUncountableEarlyExit) {
- assert(getUncountableExitingBlocks().empty() &&
- "Expected no uncountable exiting blocks");
+ if (!hasUncountableEarlyExit())
return nullptr;
- }
- assert(getUncountableExitingBlocks().size() == 1 &&
+ assert(getUncountableEdges().size() == 1 &&
"Expected only a single uncountable exiting block");
- return getUncountableExitingBlocks()[0];
+ return getUncountableEdges()[0].first;
}
/// Returns the destination of an uncountable early exiting block.
BasicBlock *getUncountableEarlyExitBlock() const {
- assert(getUncountableExitBlocks().size() == 1 &&
+ if (!hasUncountableEarlyExit())
+ return nullptr;
+ assert(getUncountableEdges().size() == 1 &&
"Expected only a single uncountable exit block");
- return getUncountableExitBlocks()[0];
+ return getUncountableEdges()[0].second;
}
/// Returns true if vector representation of the instruction \p I
@@ -463,14 +462,10 @@ class LoopVectorizationLegality {
return CountableExitingBlocks;
}
- /// Returns all the exiting blocks with an uncountable exit.
- const SmallVector<BasicBlock *, 4> &getUncountableExitingBlocks() const {
- return UncountableExitingBlocks;
- }
-
- /// Returns all the exit blocks from uncountable exiting blocks.
- SmallVector<BasicBlock *, 4> getUncountableExitBlocks() const {
- return UncountableExitBlocks;
+ /// Returns all the loop edges that have an uncountable exit.
+ const SmallVector<std::pair<BasicBlock *, BasicBlock *>, 4> &
+ getUncountableEdges() const {
+ return UncountableEdges;
}
private:
@@ -654,18 +649,13 @@ class LoopVectorizationLegality {
/// supported.
bool StructVecCallFound = false;
- /// Indicates whether this loop has an uncountable early exit, i.e. an
- /// uncountable exiting block that is not the latch.
- bool HasUncountableEarlyExit = false;
-
/// Keep track of all the countable and uncountable exiting blocks if
/// the exact backedge taken count is not computable.
SmallVector<BasicBlock *, 4> CountableExitingBlocks;
- SmallVector<BasicBlock *, 4> UncountableExitingBlocks;
- /// Keep track of the destinations of all uncountable exits if the
- /// exact backedge taken count is not computable.
- SmallVector<BasicBlock *, 4> UncountableExitBlocks;
+ /// Keep track of all the loop edges with uncountable exits, where each entry
+ /// is a pair of (Exiting, Exit) blocks.
+ SmallVector<std::pair<BasicBlock *, BasicBlock *>, 4> UncountableEdges;
};
} // namespace llvm
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
index 406864a6793dc8..00bd429a0ef9d5 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
@@ -1635,8 +1635,6 @@ bool LoopVectorizationLegality::isVectorizableEarlyExitLoop() {
const SCEV *EC =
PSE.getSE()->getPredicatedExitCount(TheLoop, BB, &Predicates);
if (isa<SCEVCouldNotCompute>(EC)) {
- UncountableExitingBlocks.push_back(BB);
-
SmallVector<BasicBlock *, 2> Succs(successors(BB));
if (Succs.size() != 2) {
reportVectorizationFailure(
@@ -1653,7 +1651,7 @@ bool LoopVectorizationLegality::isVectorizableEarlyExitLoop() {
assert(!TheLoop->contains(Succs[1]));
ExitBlock = Succs[1];
}
- UncountableExitBlocks.push_back(ExitBlock);
+ UncountableEdges.push_back({BB, ExitBlock});
} else
CountableExitingBlocks.push_back(BB);
}
@@ -1664,7 +1662,7 @@ bool LoopVectorizationLegality::isVectorizableEarlyExitLoop() {
Predicates.clear();
// We only support one uncountable early exit.
- if (getUncountableExitingBlocks().size() != 1) {
+ if (getUncountableEdges().size() != 1) {
reportVectorizationFailure(
"Loop has too many uncountable exits",
"Cannot vectorize early exit loop with more than one early exit",
@@ -1812,7 +1810,6 @@ bool LoopVectorizationLegality::canVectorize(bool UseVPlanNativePath) {
return false;
}
- HasUncountableEarlyExit = false;
if (isa<SCEVCouldNotCompute>(PSE.getBackedgeTakenCount())) {
if (TheLoop->getExitingBlock()) {
reportVectorizationFailure("Cannot vectorize uncountable loop",
@@ -1822,10 +1819,8 @@ bool LoopVectorizationLegality::canVectorize(bool UseVPlanNativePath) {
else
return false;
} else {
- HasUncountableEarlyExit = true;
if (!isVectorizableEarlyExitLoop()) {
- UncountableExitingBlocks.clear();
- HasUncountableEarlyExit = false;
+ UncountableEdges.clear();
if (DoExtraAnalysis)
Result = false;
else
|
huntergr-arm
approved these changes
Jan 16, 2025
llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
Outdated
Show resolved
Hide resolved
fhahn
reviewed
Jan 16, 2025
llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
Outdated
Show resolved
Hide resolved
fhahn
reviewed
Jan 17, 2025
| /// Returns all the exit blocks from uncountable exiting blocks. | ||
| SmallVector<BasicBlock *, 4> getUncountableExitBlocks() const { | ||
| return UncountableExitBlocks; | ||
| /// Returns the loop edge with an uncountable exit. |
There was a problem hiding this comment.
Would be good to cover also the empty case, perhaps something like
Suggested change
| /// Returns the loop edge with an uncountable exit. | |
| /// Returns the loop edge with an uncountable exit or std::nullopt if there isn't a single such edge. |
| return getUncountableExitBlocks()[0]; | ||
| if (!hasUncountableEarlyExit()) | ||
| return nullptr; | ||
| return (*getUncountableEdge()).second; |
There was a problem hiding this comment.
can this and other places just use ->?
Suggested change
| return (*getUncountableEdge()).second; | |
| return getUncountableEdge()->second; |
Comment on lines
392
to
393
| /// Returns true if the loop has an uncountable early exit, i.e. an | ||
| /// uncountable exit that isn't the latch block. |
There was a problem hiding this comment.
Might be good to clarify there must be single edge
Suggested change
| /// Returns true if the loop has an uncountable early exit, i.e. an | |
| /// uncountable exit that isn't the latch block. | |
| /// Returns true if the loop has exactly one uncountable early exit, i.e. an | |
| /// uncountable exit that isn't the latch block. |
| if (!HasUncountableEarlyExit) { | ||
| assert(getUncountableExitingBlocks().empty() && | ||
| "Expected no uncountable exiting blocks"); | ||
| if (!hasUncountableEarlyExit()) |
There was a problem hiding this comment.
nit: may be more compact
Suggested change
| if (!hasUncountableEarlyExit()) | |
| return hasUncountableEarlyExit() ? getUncountableEdge()->first : nullptr;` |
| assert(getUncountableExitingBlocks().size() == 1 && | ||
| "Expected only a single uncountable exiting block"); | ||
| return getUncountableExitingBlocks()[0]; | ||
| return (*getUncountableEdge()).first; | ||
| } | ||
|
|
||
| /// Returns the destination of an uncountable early exiting block. |
There was a problem hiding this comment.
Suggested change
| /// Returns the destination of an uncountable early exiting block. | |
| /// Returns the destination of the uncountable early exiting block, there is a single one. |
| bool hasUncountableEarlyExit() const { return HasUncountableEarlyExit; } | ||
| bool hasUncountableEarlyExit() const { | ||
| return getUncountableEdge().has_value(); | ||
| } | ||
|
|
||
| /// Returns the uncountable early exiting block. |
There was a problem hiding this comment.
Suggested change
| /// Returns the uncountable early exiting block. | |
| /// Returns the uncountable early exiting block, if there is a single one. |
| /// exact backedge taken count is not computable. | ||
| SmallVector<BasicBlock *, 4> UncountableExitBlocks; | ||
| /// Keep track of the loop edge with an uncountable exit, comprising a pair | ||
| /// of (Exiting, Exit) blocks. |
There was a problem hiding this comment.
Suggested change
| /// of (Exiting, Exit) blocks. | |
| /// of (Exiting, Exit) blocks, if there is exactly one early exit. |
fhahn
reviewed
Jan 19, 2025
| /// Returns all the exit blocks from uncountable exiting blocks. | ||
| SmallVector<BasicBlock *, 4> getUncountableExitBlocks() const { | ||
| return UncountableExitBlocks; | ||
| /// Returns the loop edge with an uncountable exit, or std::nullopt if there |
There was a problem hiding this comment.
Suggested change
| /// Returns the loop edge with an uncountable exit, or std::nullopt if there | |
| /// Returns the loop edge to an uncountable exit, or std::nullopt if there |
Not sure, but may be slightly clearer to use to?
| /// Keep track of the destinations of all uncountable exits if the | ||
| /// exact backedge taken count is not computable. | ||
| SmallVector<BasicBlock *, 4> UncountableExitBlocks; | ||
| /// Keep track of the loop edge with an uncountable exit, comprising a pair |
There was a problem hiding this comment.
Suggested change
| /// Keep track of the loop edge with an uncountable exit, comprising a pair | |
| /// Keep track of the loop edge to an uncountable exit, comprising a pair |
| @@ -1653,7 +1652,7 @@ bool LoopVectorizationLegality::isVectorizableEarlyExitLoop() { | |||
| assert(!TheLoop->contains(Succs[1])); | |||
| ExitBlock = Succs[1]; | |||
| } | |||
| UncountableExitBlocks.push_back(ExitBlock); | |||
| UncountableEdges.push_back({BB, ExitBlock}); | |||
There was a problem hiding this comment.
Simpler to exit here once we hit more than one, now that we only store one?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've removed the HasUncountableEarlyExit variable, since we can
already determine whether or not a loop has an early exit by seeing
if we found an uncountable exit.
I have also deleted the old UncountableExitingBlocks and
UncountableExitBlocks lists and replaced them with a single
uncountable edge. This means we don't need to worry about keeping the
list entries in sync and makes it clear which exiting block
corresponds to which exit block.