[LoopUnroll] UnrollRuntimeMultiExit takes precedence over TTI. #134259
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
|
@llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesUpdate UnrollRuntimeLoopRemainder to always give priority to the After ad9da92 / #124462, we would ignore the option if the backend To be consistent with canProfitablyRuntimeUnrollMultiExitLoop, always This surfaced while discussing #131998. Full diff: https://github.com/llvm/llvm-project/pull/134259.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp b/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
index 524b268aee2f3..b91f4f6c67408 100644
--- a/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
@@ -465,10 +465,6 @@ static bool canProfitablyRuntimeUnrollMultiExitLoop(
Loop *L, SmallVectorImpl<BasicBlock *> &OtherExits, BasicBlock *LatchExit,
bool UseEpilogRemainder) {
- // Priority goes to UnrollRuntimeMultiExit if it's supplied.
- if (UnrollRuntimeMultiExit.getNumOccurrences())
- return UnrollRuntimeMultiExit;
-
// The main pain point with multi-exit loop unrolling is that once unrolled,
// we will not be able to merge all blocks into a straight line code.
// There are branches within the unrolled loop that go to the OtherExits.
@@ -633,14 +629,21 @@ bool llvm::UnrollRuntimeLoopRemainder(
if (!PreserveLCSSA)
return false;
- if (!RuntimeUnrollMultiExit &&
- !canProfitablyRuntimeUnrollMultiExitLoop(L, OtherExits, LatchExit,
- UseEpilogRemainder)) {
- LLVM_DEBUG(
- dbgs()
- << "Multiple exit/exiting blocks in loop and multi-exit unrolling not "
- "enabled!\n");
- return false;
+ // Priority goes to UnrollRuntimeMultiExit if it's supplied.
+ if (UnrollRuntimeMultiExit.getNumOccurrences()) {
+ if (!UnrollRuntimeMultiExit)
+ return false;
+ } else {
+ // Otherwise perform multi-exit unrolling, if either the target indicates
+ // it is profitable or the general profitability heuristics apply.
+ if (!RuntimeUnrollMultiExit &&
+ !canProfitablyRuntimeUnrollMultiExitLoop(L, OtherExits, LatchExit,
+ UseEpilogRemainder)) {
+ LLVM_DEBUG(dbgs() << "Multiple exit/exiting blocks in loop and "
+ "multi-exit unrolling not "
+ "enabled!\n");
+ return false;
+ }
}
}
// Use Scalar Evolution to compute the trip count. This allows more loops to
diff --git a/llvm/test/Transforms/LoopUnroll/AArch64/apple-unrolling-multi-exit.ll b/llvm/test/Transforms/LoopUnroll/AArch64/apple-unrolling-multi-exit.ll
index 31b23eae0f866..23771de4c8938 100644
--- a/llvm/test/Transforms/LoopUnroll/AArch64/apple-unrolling-multi-exit.ll
+++ b/llvm/test/Transforms/LoopUnroll/AArch64/apple-unrolling-multi-exit.ll
@@ -3,6 +3,7 @@
; RUN: opt -p loop-unroll -mcpu=apple-m2 -S %s | FileCheck --check-prefix=APPLE %s
; RUN: opt -p loop-unroll -mcpu=apple-m3 -S %s | FileCheck --check-prefix=APPLE %s
; RUN: opt -p loop-unroll -mcpu=apple-m4 -S %s | FileCheck --check-prefix=APPLE %s
+; RUN: opt -p loop-unroll -mcpu=apple-m1 -unroll-runtime-multi-exit=false -S %s | FileCheck --check-prefix=OTHER %s
; RUN: opt -p loop-unroll -mcpu=cortex-a57 -S %s | FileCheck --check-prefix=OTHER %s
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-n32:64-S128-Fn32"
|
davemgreen
approved these changes
Apr 3, 2025
Comment on lines
643
to
644
| "multi-exit unrolling not " | ||
| "enabled!\n"); |
There was a problem hiding this comment.
enabled! can be combined with the previous line.
Update UnrollRuntimeLoopRemainder to always give priority to the UnrollRuntimeMultiExit option, if provided. After ad9da92, we would ignore the option if the backend indicates multi-exit is profitable. This means it cannot be used to disable runtime unrolling. To be consistent with canProfitablyRuntimeUnrollMultiExitLoop, always repsect the option. This surfaced while discussing llvm#131998.
fhahn
force-pushed
the
unroll-runtime-multi-exit-priority
branch
from
111a78e to
8fea29a
Compare
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 4, 2025
…TTI. (#134259) Update UnrollRuntimeLoopRemainder to always give priority to the UnrollRuntimeMultiExit option, if provided. After ad9da92 (llvm/llvm-project#124462), we would ignore the option if the backend indicates multi-exit is profitable. This means it cannot be used to disable runtime unrolling. To be consistent with canProfitablyRuntimeUnrollMultiExitLoop, always respect the option. This surfaced while discussing llvm/llvm-project#131998. PR: llvm/llvm-project#134259
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.
Update UnrollRuntimeLoopRemainder to always give priority to the
UnrollRuntimeMultiExit option, if provided.
After ad9da92 / #124462, we would ignore the option if the backend
indicates multi-exit is profitable. This means it cannot be used to
disable runtime unrolling.
To be consistent with canProfitablyRuntimeUnrollMultiExitLoop, always
respect the option.
This surfaced while discussing #131998.