[AArch64] Cap upper-bound unrolling of loops with uncomputable trip counts - #205102
Conversation
|
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-backend-aarch64 Author: Igor Kirillov (igogo-x86) ChangesUnrollingPreferences::MaxUpperBound does not distinguish loops with early exits from loops that run exactly 0 or N times (N <= MaxUpperBound). Bounded (upper-bound) unrolling keeps all but the last loop test, so for loops that may exit on any iteration it turns a single in-loop branch into N branches. When such loops usually exit early, unrolling replaces a single well-predicted branch with several rarely-taken ones, adding branch-predictor and code-size pressure for little benefit. Introduce UnrollingPreferences::MaxUpperBoundWithEarlyExits and the -unroll-max-upperbound-with-early-exits option to bound such loops separately. It defaults to MaxUpperBound, so behavior is unchanged unless a target or the user lowers it. Loops known to run either the max count or zero times (MaxOrZero) keep using MaxUpperBound. Set the AArch64 limit to 5 to avoid bloating small early-exit loops such as loops that count the number of bytes in a variable-length integer encoding. Full diff: https://github.com/llvm/llvm-project/pull/205102.diff 5 Files Affected:
diff --git a/llvm/include/llvm/Analysis/TargetTransformInfo.h b/llvm/include/llvm/Analysis/TargetTransformInfo.h
index aaf5ed6de93b6..3e04d2c5abc92 100644
--- a/llvm/include/llvm/Analysis/TargetTransformInfo.h
+++ b/llvm/include/llvm/Analysis/TargetTransformInfo.h
@@ -681,6 +681,9 @@ class TargetTransformInfo {
/// to be overrided by a target gives more flexiblity on certain cases.
/// By default, MaxUpperBound uses UnrollMaxUpperBound which value is 8.
unsigned MaxUpperBound;
+ /// Set the maximum upper bound of trip count for loops that may exit before
+ /// reaching the bound. This defaults to MaxUpperBound.
+ unsigned MaxUpperBoundWithEarlyExits;
/// Set the maximum unrolling factor for full unrolling. Like MaxCount, but
/// applies even if full unrolling is selected. This allows a target to fall
/// back to Partial unrolling if full unrolling is above FullUnrollMaxCount.
diff --git a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
index f8d2006cbd7de..ac367c312ed09 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
@@ -5574,6 +5574,7 @@ void AArch64TTIImpl::getUnrollingPreferences(
BaseT::getUnrollingPreferences(L, SE, UP, ORE);
UP.UpperBound = true;
+ UP.MaxUpperBoundWithEarlyExits = 5;
// For inner loop, it is more likely to be a hot one, and the runtime check
// can be promoted out from LICM pass, so the overhead is less, let's try
diff --git a/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp b/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
index 7034e595a4435..7ea7cffb3423f 100644
--- a/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
@@ -141,6 +141,11 @@ static cl::opt<unsigned> UnrollMaxUpperBound(
cl::desc(
"The max of trip count upper bound that is considered in unrolling"));
+static cl::opt<unsigned> UnrollMaxUpperBoundWithEarlyExits(
+ "unroll-max-upperbound-with-early-exits", cl::Hidden,
+ cl::desc("The max of trip count upper bound that is considered in "
+ "unrolling loops that may exit before reaching the bound"));
+
static cl::opt<unsigned> PragmaUnrollThreshold(
"pragma-unroll-threshold", cl::init(16 * 1024), cl::Hidden,
cl::desc("Unrolled size limit for loops with unroll metadata "
@@ -207,6 +212,7 @@ TargetTransformInfo::UnrollingPreferences llvm::gatherUnrollingPreferences(
UP.DefaultUnrollRuntimeCount = 8;
UP.MaxCount = std::numeric_limits<unsigned>::max();
UP.MaxUpperBound = UnrollMaxUpperBound;
+ UP.MaxUpperBoundWithEarlyExits = UP.MaxUpperBound;
UP.FullUnrollMaxCount = std::numeric_limits<unsigned>::max();
UP.BEInsns = 2;
UP.Partial = false;
@@ -247,8 +253,12 @@ TargetTransformInfo::UnrollingPreferences llvm::gatherUnrollingPreferences(
UP.MaxPercentThresholdBoost = UnrollMaxPercentThresholdBoost;
if (UnrollMaxCount.getNumOccurrences() > 0)
UP.MaxCount = UnrollMaxCount;
- if (UnrollMaxUpperBound.getNumOccurrences() > 0)
+ if (UnrollMaxUpperBound.getNumOccurrences() > 0) {
UP.MaxUpperBound = UnrollMaxUpperBound;
+ UP.MaxUpperBoundWithEarlyExits = UP.MaxUpperBound;
+ }
+ if (UnrollMaxUpperBoundWithEarlyExits.getNumOccurrences() > 0)
+ UP.MaxUpperBoundWithEarlyExits = UnrollMaxUpperBoundWithEarlyExits;
if (UnrollFullMaxCount.getNumOccurrences() > 0)
UP.FullUnrollMaxCount = UnrollFullMaxCount;
if (UnrollAllowPartial.getNumOccurrences() > 0)
@@ -1130,8 +1140,10 @@ void llvm::computeUnrollCount(Loop *L, const TargetTransformInfo &TTI,
// cost of exact full unrolling. As such, if we have an exact count and
// found it unprofitable, we'll never chose to bounded unroll.
LLVM_DEBUG(dbgs().indent(1) << "Trying upper-bound unroll...\n");
+ unsigned UpperBoundLimit =
+ MaxOrZero ? UP.MaxUpperBound : UP.MaxUpperBoundWithEarlyExits;
if (!TripCount && MaxTripCount && (UP.UpperBound || MaxOrZero) &&
- MaxTripCount <= UP.MaxUpperBound) {
+ MaxTripCount <= UpperBoundLimit) {
if (auto UnrollFactor = shouldFullUnroll(L, TTI, DT, SE, EphValues,
MaxTripCount, UCE, UP)) {
UP.Count = *UnrollFactor;
diff --git a/llvm/test/Transforms/LoopUnroll/AArch64/unroll-max-upperbound-with-early-exits.ll b/llvm/test/Transforms/LoopUnroll/AArch64/unroll-max-upperbound-with-early-exits.ll
new file mode 100644
index 0000000000000..9a034f01d76d3
--- /dev/null
+++ b/llvm/test/Transforms/LoopUnroll/AArch64/unroll-max-upperbound-with-early-exits.ll
@@ -0,0 +1,142 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes=loop-unroll -mtriple=aarch64 < %s | FileCheck %s --check-prefixes=DEFAULT
+; RUN: opt -S -passes=loop-unroll -mtriple=aarch64 -unroll-max-upperbound-with-early-exits=8 < %s | FileCheck %s --check-prefixes=WIDE
+
+; AArch64 uses a lower upper-bound limit for loops that can exit early. A typical
+; case is a varint-length helper:
+;
+; int varint_len(uint32_t serial_type) {
+; uint64_t v = serial_type;
+; int i;
+; for (i = 1; (v >>= 7) != 0; i++) {}
+; return i;
+; }
+;
+; This loop runs at most 6 times, over the AArch64 limit of 5, so it is not
+; unrolled by default. Raising the bound to 8 lets it unroll.
+define i32 @varint_i32(i32 %serial_type) {
+; DEFAULT-LABEL: @varint_i32(
+; DEFAULT-NEXT: entry:
+; DEFAULT-NEXT: [[V0:%.*]] = zext i32 [[SERIAL_TYPE:%.*]] to i64
+; DEFAULT-NEXT: br label [[LOOP:%.*]]
+; DEFAULT: loop:
+; DEFAULT-NEXT: [[I:%.*]] = phi i32 [ 1, [[ENTRY:%.*]] ], [ [[I_NEXT:%.*]], [[LOOP]] ]
+; DEFAULT-NEXT: [[V:%.*]] = phi i64 [ [[V0]], [[ENTRY]] ], [ [[V_SHR:%.*]], [[LOOP]] ]
+; DEFAULT-NEXT: [[V_SHR]] = lshr i64 [[V]], 7
+; DEFAULT-NEXT: [[I_NEXT]] = add i32 [[I]], 1
+; DEFAULT-NEXT: [[CMP:%.*]] = icmp ne i64 [[V_SHR]], 0
+; DEFAULT-NEXT: br i1 [[CMP]], label [[LOOP]], label [[EXIT:%.*]]
+; DEFAULT: exit:
+; DEFAULT-NEXT: [[I_LCSSA:%.*]] = phi i32 [ [[I]], [[LOOP]] ]
+; DEFAULT-NEXT: ret i32 [[I_LCSSA]]
+;
+; WIDE-LABEL: @varint_i32(
+; WIDE-NEXT: entry:
+; WIDE-NEXT: [[V0:%.*]] = zext i32 [[SERIAL_TYPE:%.*]] to i64
+; WIDE-NEXT: br label [[LOOP:%.*]]
+; WIDE: loop:
+; WIDE-NEXT: [[V_SHR:%.*]] = lshr i64 [[V0]], 7
+; WIDE-NEXT: [[CMP:%.*]] = icmp ne i64 [[V_SHR]], 0
+; WIDE-NEXT: br i1 [[CMP]], label [[LOOP_1:%.*]], label [[EXIT:%.*]]
+; WIDE: loop.1:
+; WIDE-NEXT: [[V_SHR_1:%.*]] = lshr i64 [[V_SHR]], 7
+; WIDE-NEXT: [[CMP_1:%.*]] = icmp ne i64 [[V_SHR_1]], 0
+; WIDE-NEXT: br i1 [[CMP_1]], label [[LOOP_2:%.*]], label [[EXIT]]
+; WIDE: loop.2:
+; WIDE-NEXT: [[V_SHR_2:%.*]] = lshr i64 [[V_SHR_1]], 7
+; WIDE-NEXT: [[CMP_2:%.*]] = icmp ne i64 [[V_SHR_2]], 0
+; WIDE-NEXT: br i1 [[CMP_2]], label [[LOOP_3:%.*]], label [[EXIT]]
+; WIDE: loop.3:
+; WIDE-NEXT: [[V_SHR_3:%.*]] = lshr i64 [[V_SHR_2]], 7
+; WIDE-NEXT: [[CMP_3:%.*]] = icmp ne i64 [[V_SHR_3]], 0
+; WIDE-NEXT: br i1 [[CMP_3]], label [[LOOP_4:%.*]], label [[EXIT]]
+; WIDE: loop.4:
+; WIDE-NEXT: [[V_SHR_4:%.*]] = lshr i64 [[V_SHR_3]], 7
+; WIDE-NEXT: [[CMP_4:%.*]] = icmp ne i64 [[V_SHR_4]], 0
+; WIDE-NEXT: br i1 [[CMP_4]], label [[LOOP_5:%.*]], label [[EXIT]]
+; WIDE: loop.5:
+; WIDE-NEXT: br label [[EXIT]]
+; WIDE: exit:
+; WIDE-NEXT: [[I_LCSSA:%.*]] = phi i32 [ 1, [[LOOP]] ], [ 2, [[LOOP_1]] ], [ 3, [[LOOP_2]] ], [ 4, [[LOOP_3]] ], [ 5, [[LOOP_4]] ], [ 6, [[LOOP_5]] ]
+; WIDE-NEXT: ret i32 [[I_LCSSA]]
+;
+entry:
+ %v0 = zext i32 %serial_type to i64
+ br label %loop
+
+loop:
+ %i = phi i32 [ 1, %entry ], [ %i.next, %loop ]
+ %v = phi i64 [ %v0, %entry ], [ %v.shr, %loop ]
+ %v.shr = lshr i64 %v, 7
+ %i.next = add i32 %i, 1
+ %cmp = icmp ne i64 %v.shr, 0
+ br i1 %cmp, label %loop, label %exit
+
+exit:
+ %i.lcssa = phi i32 [ %i, %loop ]
+ ret i32 %i.lcssa
+}
+
+; The 16-bit version runs at most 4 times, under the limit of 5, so it is still
+; unrolled by default.
+define i32 @varint_i16(i16 %serial_type) {
+; DEFAULT-LABEL: @varint_i16(
+; DEFAULT-NEXT: entry:
+; DEFAULT-NEXT: [[V0:%.*]] = zext i16 [[SERIAL_TYPE:%.*]] to i64
+; DEFAULT-NEXT: br label [[LOOP:%.*]]
+; DEFAULT: loop:
+; DEFAULT-NEXT: [[V_SHR:%.*]] = lshr i64 [[V0]], 7
+; DEFAULT-NEXT: [[CMP:%.*]] = icmp ne i64 [[V_SHR]], 0
+; DEFAULT-NEXT: br i1 [[CMP]], label [[LOOP_1:%.*]], label [[EXIT:%.*]]
+; DEFAULT: loop.1:
+; DEFAULT-NEXT: [[V_SHR_1:%.*]] = lshr i64 [[V_SHR]], 7
+; DEFAULT-NEXT: [[CMP_1:%.*]] = icmp ne i64 [[V_SHR_1]], 0
+; DEFAULT-NEXT: br i1 [[CMP_1]], label [[LOOP_2:%.*]], label [[EXIT]]
+; DEFAULT: loop.2:
+; DEFAULT-NEXT: [[V_SHR_2:%.*]] = lshr i64 [[V_SHR_1]], 7
+; DEFAULT-NEXT: [[CMP_2:%.*]] = icmp ne i64 [[V_SHR_2]], 0
+; DEFAULT-NEXT: br i1 [[CMP_2]], label [[LOOP_3:%.*]], label [[EXIT]]
+; DEFAULT: loop.3:
+; DEFAULT-NEXT: br label [[EXIT]]
+; DEFAULT: exit:
+; DEFAULT-NEXT: [[I_LCSSA:%.*]] = phi i32 [ 1, [[LOOP]] ], [ 2, [[LOOP_1]] ], [ 3, [[LOOP_2]] ], [ 4, [[LOOP_3]] ]
+; DEFAULT-NEXT: ret i32 [[I_LCSSA]]
+;
+; WIDE-LABEL: @varint_i16(
+; WIDE-NEXT: entry:
+; WIDE-NEXT: [[V0:%.*]] = zext i16 [[SERIAL_TYPE:%.*]] to i64
+; WIDE-NEXT: br label [[LOOP:%.*]]
+; WIDE: loop:
+; WIDE-NEXT: [[V_SHR:%.*]] = lshr i64 [[V0]], 7
+; WIDE-NEXT: [[CMP:%.*]] = icmp ne i64 [[V_SHR]], 0
+; WIDE-NEXT: br i1 [[CMP]], label [[LOOP_1:%.*]], label [[EXIT:%.*]]
+; WIDE: loop.1:
+; WIDE-NEXT: [[V_SHR_1:%.*]] = lshr i64 [[V_SHR]], 7
+; WIDE-NEXT: [[CMP_1:%.*]] = icmp ne i64 [[V_SHR_1]], 0
+; WIDE-NEXT: br i1 [[CMP_1]], label [[LOOP_2:%.*]], label [[EXIT]]
+; WIDE: loop.2:
+; WIDE-NEXT: [[V_SHR_2:%.*]] = lshr i64 [[V_SHR_1]], 7
+; WIDE-NEXT: [[CMP_2:%.*]] = icmp ne i64 [[V_SHR_2]], 0
+; WIDE-NEXT: br i1 [[CMP_2]], label [[LOOP_3:%.*]], label [[EXIT]]
+; WIDE: loop.3:
+; WIDE-NEXT: br label [[EXIT]]
+; WIDE: exit:
+; WIDE-NEXT: [[I_LCSSA:%.*]] = phi i32 [ 1, [[LOOP]] ], [ 2, [[LOOP_1]] ], [ 3, [[LOOP_2]] ], [ 4, [[LOOP_3]] ]
+; WIDE-NEXT: ret i32 [[I_LCSSA]]
+;
+entry:
+ %v0 = zext i16 %serial_type to i64
+ br label %loop
+
+loop:
+ %i = phi i32 [ 1, %entry ], [ %i.next, %loop ]
+ %v = phi i64 [ %v0, %entry ], [ %v.shr, %loop ]
+ %v.shr = lshr i64 %v, 7
+ %i.next = add i32 %i, 1
+ %cmp = icmp ne i64 %v.shr, 0
+ br i1 %cmp, label %loop, label %exit
+
+exit:
+ %i.lcssa = phi i32 [ %i, %loop ]
+ ret i32 %i.lcssa
+}
diff --git a/llvm/test/Transforms/LoopUnroll/unroll-max-upperbound-with-early-exits.ll b/llvm/test/Transforms/LoopUnroll/unroll-max-upperbound-with-early-exits.ll
new file mode 100644
index 0000000000000..29ff1345cfd07
--- /dev/null
+++ b/llvm/test/Transforms/LoopUnroll/unroll-max-upperbound-with-early-exits.ll
@@ -0,0 +1,137 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes='loop-unroll<upperbound>' < %s | FileCheck %s --check-prefixes=DEFAULT
+; RUN: opt -S -passes='loop-unroll<upperbound>' -unroll-max-upperbound-with-early-exits=5 < %s | FileCheck %s --check-prefixes=LIMIT
+
+; -unroll-max-upperbound-with-early-exits only limits loops that can exit early.
+; Loops that run their max count or zero times still use -unroll-max-upperbound.
+
+; This loop can exit on any iteration and runs at most 6 times. The bound of 5
+; is below that, so it is not unrolled.
+define i32 @early_exit(i32 %serial_type) {
+; DEFAULT-LABEL: @early_exit(
+; DEFAULT-NEXT: entry:
+; DEFAULT-NEXT: [[V0:%.*]] = zext i32 [[SERIAL_TYPE:%.*]] to i64
+; DEFAULT-NEXT: br label [[LOOP:%.*]]
+; DEFAULT: loop:
+; DEFAULT-NEXT: [[V_SHR:%.*]] = lshr i64 [[V0]], 7
+; DEFAULT-NEXT: [[CMP:%.*]] = icmp ne i64 [[V_SHR]], 0
+; DEFAULT-NEXT: br i1 [[CMP]], label [[LOOP_1:%.*]], label [[EXIT:%.*]]
+; DEFAULT: loop.1:
+; DEFAULT-NEXT: [[V_SHR_1:%.*]] = lshr i64 [[V_SHR]], 7
+; DEFAULT-NEXT: [[CMP_1:%.*]] = icmp ne i64 [[V_SHR_1]], 0
+; DEFAULT-NEXT: br i1 [[CMP_1]], label [[LOOP_2:%.*]], label [[EXIT]]
+; DEFAULT: loop.2:
+; DEFAULT-NEXT: [[V_SHR_2:%.*]] = lshr i64 [[V_SHR_1]], 7
+; DEFAULT-NEXT: [[CMP_2:%.*]] = icmp ne i64 [[V_SHR_2]], 0
+; DEFAULT-NEXT: br i1 [[CMP_2]], label [[LOOP_3:%.*]], label [[EXIT]]
+; DEFAULT: loop.3:
+; DEFAULT-NEXT: [[V_SHR_3:%.*]] = lshr i64 [[V_SHR_2]], 7
+; DEFAULT-NEXT: [[CMP_3:%.*]] = icmp ne i64 [[V_SHR_3]], 0
+; DEFAULT-NEXT: br i1 [[CMP_3]], label [[LOOP_4:%.*]], label [[EXIT]]
+; DEFAULT: loop.4:
+; DEFAULT-NEXT: [[V_SHR_4:%.*]] = lshr i64 [[V_SHR_3]], 7
+; DEFAULT-NEXT: [[CMP_4:%.*]] = icmp ne i64 [[V_SHR_4]], 0
+; DEFAULT-NEXT: br i1 [[CMP_4]], label [[LOOP_5:%.*]], label [[EXIT]]
+; DEFAULT: loop.5:
+; DEFAULT-NEXT: br label [[EXIT]]
+; DEFAULT: exit:
+; DEFAULT-NEXT: [[I_LCSSA:%.*]] = phi i32 [ 1, [[LOOP]] ], [ 2, [[LOOP_1]] ], [ 3, [[LOOP_2]] ], [ 4, [[LOOP_3]] ], [ 5, [[LOOP_4]] ], [ 6, [[LOOP_5]] ]
+; DEFAULT-NEXT: ret i32 [[I_LCSSA]]
+;
+; LIMIT-LABEL: @early_exit(
+; LIMIT-NEXT: entry:
+; LIMIT-NEXT: [[V0:%.*]] = zext i32 [[SERIAL_TYPE:%.*]] to i64
+; LIMIT-NEXT: br label [[LOOP:%.*]]
+; LIMIT: loop:
+; LIMIT-NEXT: [[I:%.*]] = phi i32 [ 1, [[ENTRY:%.*]] ], [ [[I_NEXT:%.*]], [[LOOP]] ]
+; LIMIT-NEXT: [[V:%.*]] = phi i64 [ [[V0]], [[ENTRY]] ], [ [[V_SHR:%.*]], [[LOOP]] ]
+; LIMIT-NEXT: [[V_SHR]] = lshr i64 [[V]], 7
+; LIMIT-NEXT: [[I_NEXT]] = add i32 [[I]], 1
+; LIMIT-NEXT: [[CMP:%.*]] = icmp ne i64 [[V_SHR]], 0
+; LIMIT-NEXT: br i1 [[CMP]], label [[LOOP]], label [[EXIT:%.*]]
+; LIMIT: exit:
+; LIMIT-NEXT: [[I_LCSSA:%.*]] = phi i32 [ [[I]], [[LOOP]] ]
+; LIMIT-NEXT: ret i32 [[I_LCSSA]]
+;
+entry:
+ %v0 = zext i32 %serial_type to i64
+ br label %loop
+
+loop:
+ %i = phi i32 [ 1, %entry ], [ %i.next, %loop ]
+ %v = phi i64 [ %v0, %entry ], [ %v.shr, %loop ]
+ %v.shr = lshr i64 %v, 7
+ %i.next = add i32 %i, 1
+ %cmp = icmp ne i64 %v.shr, 0
+ br i1 %cmp, label %loop, label %exit
+
+exit:
+ %i.lcssa = phi i32 [ %i, %loop ]
+ ret i32 %i.lcssa
+}
+
+; This loop runs at most 6 times, but always its max count or zero times, so the
+; option does not affect it. It is fully unrolled in both runs.
+define i32 @max_or_zero(i32 %n, ptr %p) {
+; DEFAULT-LABEL: @max_or_zero(
+; DEFAULT-NEXT: entry:
+; DEFAULT-NEXT: [[END:%.*]] = add i32 [[N:%.*]], 6
+; DEFAULT-NEXT: br label [[LOOP:%.*]]
+; DEFAULT: loop:
+; DEFAULT-NEXT: store i32 [[N]], ptr [[P:%.*]], align 4
+; DEFAULT-NEXT: [[I_NEXT:%.*]] = add i32 [[N]], 1
+; DEFAULT-NEXT: [[C:%.*]] = icmp ult i32 [[I_NEXT]], [[END]]
+; DEFAULT-NEXT: br i1 [[C]], label [[LOOP_1:%.*]], label [[EXIT:%.*]]
+; DEFAULT: loop.1:
+; DEFAULT-NEXT: store i32 [[I_NEXT]], ptr [[P]], align 4
+; DEFAULT-NEXT: [[I_NEXT_1:%.*]] = add i32 [[N]], 2
+; DEFAULT-NEXT: store i32 [[I_NEXT_1]], ptr [[P]], align 4
+; DEFAULT-NEXT: [[I_NEXT_2:%.*]] = add i32 [[N]], 3
+; DEFAULT-NEXT: store i32 [[I_NEXT_2]], ptr [[P]], align 4
+; DEFAULT-NEXT: [[I_NEXT_3:%.*]] = add i32 [[N]], 4
+; DEFAULT-NEXT: store i32 [[I_NEXT_3]], ptr [[P]], align 4
+; DEFAULT-NEXT: [[I_NEXT_4:%.*]] = add i32 [[N]], 5
+; DEFAULT-NEXT: store i32 [[I_NEXT_4]], ptr [[P]], align 4
+; DEFAULT-NEXT: br label [[EXIT]]
+; DEFAULT: exit:
+; DEFAULT-NEXT: [[I_LCSSA:%.*]] = phi i32 [ [[N]], [[LOOP]] ], [ [[I_NEXT_4]], [[LOOP_1]] ]
+; DEFAULT-NEXT: ret i32 [[I_LCSSA]]
+;
+; LIMIT-LABEL: @max_or_zero(
+; LIMIT-NEXT: entry:
+; LIMIT-NEXT: [[END:%.*]] = add i32 [[N:%.*]], 6
+; LIMIT-NEXT: br label [[LOOP:%.*]]
+; LIMIT: loop:
+; LIMIT-NEXT: store i32 [[N]], ptr [[P:%.*]], align 4
+; LIMIT-NEXT: [[I_NEXT:%.*]] = add i32 [[N]], 1
+; LIMIT-NEXT: [[C:%.*]] = icmp ult i32 [[I_NEXT]], [[END]]
+; LIMIT-NEXT: br i1 [[C]], label [[LOOP_1:%.*]], label [[EXIT:%.*]]
+; LIMIT: loop.1:
+; LIMIT-NEXT: store i32 [[I_NEXT]], ptr [[P]], align 4
+; LIMIT-NEXT: [[I_NEXT_1:%.*]] = add i32 [[N]], 2
+; LIMIT-NEXT: store i32 [[I_NEXT_1]], ptr [[P]], align 4
+; LIMIT-NEXT: [[I_NEXT_2:%.*]] = add i32 [[N]], 3
+; LIMIT-NEXT: store i32 [[I_NEXT_2]], ptr [[P]], align 4
+; LIMIT-NEXT: [[I_NEXT_3:%.*]] = add i32 [[N]], 4
+; LIMIT-NEXT: store i32 [[I_NEXT_3]], ptr [[P]], align 4
+; LIMIT-NEXT: [[I_NEXT_4:%.*]] = add i32 [[N]], 5
+; LIMIT-NEXT: store i32 [[I_NEXT_4]], ptr [[P]], align 4
+; LIMIT-NEXT: br label [[EXIT]]
+; LIMIT: exit:
+; LIMIT-NEXT: [[I_LCSSA:%.*]] = phi i32 [ [[N]], [[LOOP]] ], [ [[I_NEXT_4]], [[LOOP_1]] ]
+; LIMIT-NEXT: ret i32 [[I_LCSSA]]
+;
+entry:
+ %end = add i32 %n, 6
+ br label %loop
+
+loop:
+ %i = phi i32 [ %n, %entry ], [ %i.next, %loop ]
+ store i32 %i, ptr %p
+ %i.next = add i32 %i, 1
+ %c = icmp ult i32 %i.next, %end
+ br i1 %c, label %loop, label %exit
+
+exit:
+ ret i32 %i
+}
|
d607f96 to
94a0b64
Compare
Why is |
| BaseT::getUnrollingPreferences(L, SE, UP, ORE); | ||
|
|
||
| UP.UpperBound = true; | ||
| UP.MaxUpperBoundUnknownTripCount = 5; |
There was a problem hiding this comment.
Why 5? Does this correspond to the branch predictor, or is this cherry-picked for sqlite3?
There was a problem hiding this comment.
Yep, it's cherry-picked for that particular case, and I didn't see any other regression caused by it
94a0b64 to
975eacc
Compare
We see a regression of about 3% in sqlite3 from SPEC CPU 2026 on AArch64, and we cannot change the source code there. The regression was exposed by #197292. That patch is an improvement from a general compiler perspective. Before it, however, the maximum trip count for sqlite3’s varint loops:
was far above The SCEV change is correct, so I came up with a fix that adds an unrolling heuristic that treats counted loops and data-dependent loops differently, since the latter are less profitable to unroll. |
justinfargnoli
left a comment
There was a problem hiding this comment.
Thanks for iterating on this @igogo-x86!
Please get approval from someone on the AArch64 side of things before merging.
|
What does this PR look like with after regenerating all the tests so they pass? |
|
@david-arm This comment shows the extra we would have to put into this patch if we don't land #208500 |
Maybe I'm missing something, but that diff looks great. It basically says you've reduce the amount of generated IR and CSE'd some calls to vscale, right? |
Well, yes, but the premise behind it isn’t sound, and the change is outside the scope of this patch. If you are fine - I can just change these tests in this patch and we are done. |
975eacc to
8e05eb2
Compare
|
That's what PR looks like without #208500 (and with comments addressed) |
|
Right I understand this a bit more now. Thanks for showing the full diff here @igogo-x86 - it really helps highlight the problem! This PR exposed an underlying bug with the vectoriser where it's not safe to pass around ScalarEvolution pointers after forgetting the loop. If we pass the #208500 fixes the underlying bug exposed by this PR and once it lands the extra loop vectoriser test diffs in this PR will disappear. |
LoopVectorize decides whether to attach llvm.loop.unroll.runtime.disable metadata to the vector loop based on a single target-wide bit, UnrollingPreferences::UnrollVectorizedLoop (AMDGPU sets it unconditionally since D149281). To read this bit, it called getUnrollingPreferences on the newly created vector loop and passed the vectorizer's ScalarEvolution, so SCEV was queried on IR in the middle of the transform. This has a visible side effect: any SCEV query at this point fills the SCEV caches with expressions for the new loop skeleton, and this changes which existing values SCEVExpander reuses later when it expands the minimum-iteration checks for the epilogue. As a result, if a target's getUnrollingPreferences queries SCEV (ARM already does, AArch64 will in llvm#205102), the emitted IR depends on when the hook runs. UnrollVectorizedLoop is a target constant, so query it earlier, on the original loop - before the plan executes and before the original loop's SCEV is forgotten. At that point the query is a cache hit and does not add any new vector-loop expressions to the caches.
…zedLoop (#208500) LoopVectorize queried getUnrollingPreferences after VPlan execution, when the vector loop had already been created. If the target hook queries ScalarEvolution, this populates the SCEV caches with expressions for the new loop and changes which existing values SCEVExpander reuses later. This is exposed by #205102, where the new AArch64 unrolling preferences cause unrelated LoopVectorize tests to produce different IR. UnrollVectorizedLoop is a target-wide preference. Query it on the original loop before VPlan execution and before its SCEV information is forgotten. This prevents the preference query from polluting subsequent SCEV expansion.
…zedLoop (llvm#208500) LoopVectorize queried getUnrollingPreferences after VPlan execution, when the vector loop had already been created. If the target hook queries ScalarEvolution, this populates the SCEV caches with expressions for the new loop and changes which existing values SCEVExpander reuses later. This is exposed by llvm#205102, where the new AArch64 unrolling preferences cause unrelated LoopVectorize tests to produce different IR. UnrollVectorizedLoop is a target-wide preference. Query it on the original loop before VPlan execution and before its SCEV information is forgotten. This prevents the preference query from polluting subsequent SCEV expansion.
8e05eb2 to
c6fcba8
Compare
| // Also disable runtime unrolling, which would clamp the unroll count to the | ||
| // known maximum trip count and produce the same complete unroll. | ||
| if (L->getExitingBlock() && !SE.isBackedgeTakenCountMaxOrZero(L) && | ||
| isa<SCEVCouldNotCompute>(SE.getBackedgeTakenCount(L))) { |
There was a problem hiding this comment.
Does isa<SCEVCouldNotCompute>(SE.getBackedgeTakenCount(L)) already imply that SE.isBackedgeTakenCountMaxOrZero(L) is false? If we cannot compute the backedge taken count how could we know that it's max or zero?
There was a problem hiding this comment.
I had a look and it looks like yes, you are right. The only place SCEV sets MaxOrZero = true is howManyLessThans:
} else if (BECountIfBackedgeTaken &&
isa<SCEVConstant>(BECountIfBackedgeTaken)) {
// If we know exactly how many times the backedge will be taken if it's
// taken at least once, then the backedge count will either be that or
// zero.
ConstantMaxBECount = BECountIfBackedgeTaken;
MaxOrZero = true;
and every path reaching this has already computed the exact BECount. However, it doesn't look guaranteed by API and may theoretically change in the future? Anyway, I am happy to drop it if you'd rather keep the condition minimal.
There was a problem hiding this comment.
OK. I think this check may also apply to some forms of early exit loops like this:
while (i < n) {
if (val1[i] == val2[i])
break;
i++;
}
although I'm not sure what the types of loops the unroller currently supports. This is because sometimes the early exit check and the latch checks can be fused together into a single check so that there is only a single exiting exiting block, which means getExitingBlock returns non-null. In this case the backedge taken count cannot also not be computed and we may end up disabling runtime unrolling for such loops too. However, as far as I understand no performance regressions have been observed with this PR so we can give it a try. I think in general I agree with the principle of not speculatively unrolling the latch checks due to the extra code growth without necessarily any performance benefit.
| ; int varint_len(uint32_t serial_type) { | ||
| ; uint64_t v = serial_type; | ||
| ; int i; | ||
| ; for (i = 1; (v >>= 7) != 0; i++) {} |
There was a problem hiding this comment.
A part of me wonders if this is so common we could also just recognise this as a loop idiom and have a new intrinsic for it? That would stop the unrolling and enable targets to generate the best possible code.
There was a problem hiding this comment.
For this particular shape, no new intrinsic is needed - we already have llvm.ctlz and can rewrite this as:
len = (BW + K - 1 - ctlz(v | 1)) / K
or in sqlite3 case:
(38 - clz32(serial_type | 1)) / 7
However, it was slower because it results in several instructions (it's much faster on random data though)
| @@ -0,0 +1,217 @@ | |||
| ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6 | |||
| ; RUN: opt -S -passes=loop-unroll -mtriple=aarch64 < %s | FileCheck %s --check-prefixes=DEFAULT | |||
| ; RUN: opt -S -passes=loop-unroll -mtriple=aarch64 -unroll-max-upperbound=8 < %s | FileCheck %s --check-prefixes=WIDE | |||
There was a problem hiding this comment.
Probably worth adding a RUN line when forcing UP.Runtime=true via the command line, if that's possible.
| // A loop can have a small maximum trip count while SCEV still cannot | ||
| // form an exact backedge count - typically a data-dependent exit, e.g. | ||
| // shifting a value until it reaches zero. Unrolling such a loop trades one | ||
| // well-predicted backedge for a chain of rarely-taken exit branches, so hold |
There was a problem hiding this comment.
Surely this is only true for some loops (presumably including the one you care about), but in the general case it may not be. In your varint_len example below what if v=1? If we unroll such a loop then every early exit would be taken, so it would be a chain of always-taken exit branches instead? i.e.
if ((1 >> 7) == 0) break;
if ((1 >> 14) == 0) break;
etc.
This comment suggests we are making assumptions about the input data and using that as the motivation for the change.
There was a problem hiding this comment.
Agreed, reworded to make clear this is a conservative heuristic, not a claim that unrolling always loses. About v=1: at runtime, both versions execute a single conditional branch (the unrolled body leaves through the first exit test and the rest are never reached), so the cost there is static - six branch sites and the code growth instead of one. Where behaviour really diverges is when the length varies from call to call, and whether unrolling then helps depends on the distribution, which we can't know at compile time. The code growth is certain, and it measurably regressed sqlite3. Loops where a profile says otherwise can still be peeled (profile-guided peeling is deliberately unaffected)
…arly exits UnrollingPreferences::MaxUpperBound does not distinguish loops with early exits from loops that run exactly 0 or N times (N <= MaxUpperBound). Bounded (upper-bound) unrolling keeps all but the last loop test, so for loops that may exit on any iteration it turns a single in-loop branch into N branches. When such loops usually exit early, unrolling replaces a single well-predicted branch with several rarely-taken ones, adding branch-predictor and code-size pressure for little benefit. Introduce UnrollingPreferences::MaxUpperBoundWithEarlyExits and the -unroll-max-upperbound-with-early-exits option to bound such loops separately. It defaults to MaxUpperBound, so behavior is unchanged unless a target or the user lowers it. Loops known to run either the max count or zero times (MaxOrZero) keep using MaxUpperBound. Set the AArch64 limit to 5 to avoid bloating small early-exit loops such as loops that count the number of bytes in a variable-length integer encoding.
Classify loop preciseily - single exiting block, !MaxOrZero and an exact backedge count that is SCEVCouldNotTcompute.
c6fcba8 to
dee6c39
Compare
| @@ -0,0 +1,221 @@ | |||
| ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6 | |||
| ; RUN: opt -S -passes=loop-unroll -mtriple=aarch64 < %s | FileCheck %s --check-prefixes=DEFAULT | |||
There was a problem hiding this comment.
nit: Just a suggestion, but if you change all the RUN lines to use --check-prefixes=COMMON,... then in places where DEFAULT==WIDE it only generates a single set of COMMON check lines.
| // Also disable runtime unrolling, which would clamp the unroll count to the | ||
| // known maximum trip count and produce the same complete unroll. | ||
| if (L->getExitingBlock() && !SE.isBackedgeTakenCountMaxOrZero(L) && | ||
| isa<SCEVCouldNotCompute>(SE.getBackedgeTakenCount(L))) { |
There was a problem hiding this comment.
OK. I think this check may also apply to some forms of early exit loops like this:
while (i < n) {
if (val1[i] == val2[i])
break;
i++;
}
although I'm not sure what the types of loops the unroller currently supports. This is because sometimes the early exit check and the latch checks can be fused together into a single check so that there is only a single exiting exiting block, which means getExitingBlock returns non-null. In this case the backedge taken count cannot also not be computed and we may end up disabling runtime unrolling for such loops too. However, as far as I understand no performance regressions have been observed with this PR so we can give it a try. I think in general I agree with the principle of not speculatively unrolling the latch checks due to the extra code growth without necessarily any performance benefit.
SCEV can compute a small maximum trip count for a loop while still being
unable to form an exact backedge-taken count (getBackedgeTakenCount
returns SCEVCouldNotCompute). This is typical of data-dependent exits,
such as the varint-length loops found in sqlite3:
for (i = 1; (v >>= 7) != 0; i++)
;
Because the exit is data-dependent, the iteration count is not known at
compile time. Unrolling up to the maximum trip count turns a single
well-predicted backedge into a chain of copies, each with its own exit
test - growing code size and the static branch count - without any
guarantee that enough iterations run to pay for it.
These loops were not unrolled before #197292, which tightened SCEV's
maximum backedge-taken count for shift-recurrence loops. That change is a
general improvement, but it lowered the computed maximum trip count of
these loops (6 for a 32-bit varint) below the default MaxUpperBound of 8,
making them eligible for upper-bound unrolling and regressing sqlite3 by
~3% on SPEC CPU 2026.
For single-exit loops that are not MaxOrZero and whose exact backedge
count is SCEVCouldNotCompute, lower MaxUpperBound to 5 (which still lets
smaller early-exit loops unroll) and disable runtime unrolling. Runtime
unrolling is disabled too because, with a known small maximum trip count,
it would clamp the unroll count to that maximum and produce the same
complete unroll.