From 9f1b6b668c0d852ffe6847d5992a2d31f0e23dd3 Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" Date: Mon, 3 Nov 2025 18:04:07 -0500 Subject: [PATCH 1/3] [LoopUnroll] Fix division by zero PR #159163's probability computation for epilogue loops does not handle the possibility of an original loop probability of one. Runtime loop unrolling does not make sense for such an infinite loop, and a division by zero results. This patch works around that case. Issue #165998. --- .../Transforms/Utils/LoopUnrollRuntime.cpp | 9 ++ .../LoopUnroll/loop-probability-one.ll | 114 ++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 llvm/test/Transforms/LoopUnroll/loop-probability-one.ll diff --git a/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp b/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp index 1e8f6cc76900c..1599c743c51cf 100644 --- a/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp +++ b/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp @@ -200,8 +200,17 @@ static void ConnectProlog(Loop *L, Value *BECount, unsigned Count, /// from 0 to \p N more iterations can possibly execute. Among such cases in /// the original loop (with loop probability \p OriginalLoopProb), what is the /// probability of executing at least one more iteration? +/// +/// If \p OriginalLoopProb is 1, then the original loop was somehow determined +/// to be always infinite. Runtime loop unrolling should be impossible for that +/// case. What is infinity % UnrollCount? But we might have bad profile data. +/// In that case, we arbitrarily choose to keep the probability at 1 throughout +/// the remainder loop. static BranchProbability probOfNextInRemainder(BranchProbability OriginalLoopProb, unsigned N) { + if (OriginalLoopProb == BranchProbability::getOne()) + return BranchProbability::getOne(); + // Each of these variables holds the original loop's probability that the // number of iterations it will execute is some m in the specified range. BranchProbability ProbOne = OriginalLoopProb; // 1 <= m diff --git a/llvm/test/Transforms/LoopUnroll/loop-probability-one.ll b/llvm/test/Transforms/LoopUnroll/loop-probability-one.ll new file mode 100644 index 0000000000000..bdd2aaf5b7f4e --- /dev/null +++ b/llvm/test/Transforms/LoopUnroll/loop-probability-one.ll @@ -0,0 +1,114 @@ +; Check that a loop probability of one (indicating an always infinite loop) does +; not crash or otherwise break LoopUnroll behavior when it tries to compute new +; probabilities from it. +; +; Runtime loop unrolling should be impossible for that case. What is infinity % +; UnrollCount? But we can have bad profile data. In that case, the +; implementation arbitrarily chooses to keep the probability at 1 throughout the +; remainder loop. + +; DEFINE: %{unroll} = opt < %s -unroll-count=3 -passes=loop-unroll -S +; DEFINE: %{rt} = %{unroll} -unroll-runtime + +; RUN: %{unroll} | FileCheck %s -check-prefix UNROLL +; RUN: %{rt} -unroll-runtime-epilog=true | FileCheck %s -check-prefix EPILOG +; RUN: %{rt} -unroll-runtime-epilog=false | FileCheck %s -check-prefix PROLOG + +define void @test(i32 %n) { +entry: + br label %loop + +loop: + %i = phi i32 [ 0, %entry ], [ %inc, %loop ] + %inc = add i32 %i, 1 + %c = icmp slt i32 %inc, %n + br i1 %c, label %loop, label %end, !prof !0 + +end: + ret void +} + + +!0 = !{!"branch_weights", i32 1, i32 0} + +; UNROLL: define void @test(i32 %n) { +; UNROLL: entry: +; UNROLL: br label %loop +; UNROLL: loop: +; UNROLL: br i1 %c, label %loop.1, label %end, !prof !0 +; UNROLL: loop.1: +; UNROLL: br i1 %c.1, label %loop.2, label %end, !prof !0 +; UNROLL: loop.2: +; UNROLL: br i1 %c.2, label %loop, label %end, !prof !0, !llvm.loop !1 +; UNROLL-NOT: loop.3 +; UNROLL: end: +; UNROLL: ret void +; UNROLL: } +; +; Infinite unrolled loop. +; UNROLL: !0 = !{!"branch_weights", i32 1, i32 0} + +; EPILOG: define void @test(i32 %n) { +; EPILOG: entry: +; EPILOG: br i1 %{{.*}}, label %loop.epil.preheader, label %entry.new, !prof !0 +; EPILOG: entry.new: +; EPILOG: br label %loop +; EPILOG: loop: +; EPILOG: br i1 %{{.*}}, label %loop, label %end.unr-lcssa, !prof !1 +; EPILOG: end.unr-lcssa: +; EPILOG: br i1 %{{.*}}, label %loop.epil.preheader, label %end, !prof !1 +; EPILOG: loop.epil.preheader: +; EPILOG: br label %loop.epil +; EPILOG: loop.epil: +; EPILOG: br i1 %{{.*}}, label %loop.epil, label %end.epilog-lcssa, !prof !4 +; EPILOG: end.epilog-lcssa: +; EPILOG: br label %end +; EPILOG: end: +; EPILOG: ret void +; EPILOG: } +; +; Unrolled loop guard: Unrolled loop is always entered. +; EPILOG: !0 = !{!"branch_weights", i32 0, i32 -2147483648} +; +; Unrolled loop latch: Unrolled loop is infinite. +; Epilogue loop guard: Epilogue loop is always entered if unrolled loop exits. +; EPILOG: !1 = !{!"branch_weights", i32 -2147483648, i32 0} +; +; Epilogue loop latch: Epilogue loop executes both of its 2 iterations. +; EPILOG: !4 = !{!"branch_weights", i32 1073741824, i32 1073741824} + +; PROLOG: define void @test(i32 %n) { +; PROLOG: entry: +; PROLOG: br i1 %{{.*}}, label %loop.prol.preheader, label %loop.prol.loopexit, !prof !0 +; PROLOG: loop.prol.preheader: +; PROLOG: br label %loop.prol +; PROLOG: loop.prol: +; PROLOG: br i1 %{{.*}}, label %loop.prol, label %loop.prol.loopexit.unr-lcssa, !prof !1 +; PROLOG: loop.prol.loopexit.unr-lcssa: +; PROLOG: br label %loop.prol.loopexit +; PROLOG: loop.prol.loopexit: +; PROLOG: br i1 %{{.*}}, label %end, label %entry.new, !prof !0 +; PROLOG: entry.new: +; PROLOG: br label %loop +; PROLOG: loop: +; PROLOG: br i1 %{{.*}}, label %loop, label %end.unr-lcssa, !prof !4 +; PROLOG: end.unr-lcssa: +; PROLOG: br label %end +; PROLOG: end: +; PROLOG: ret void +; PROLOG: } +; +; FIXME: Branch weights still need to be fixed in the case of prologues (issue +; #135812), so !0 and !1 do not yet match their comments below. When we do +; fix it, this test will hopefully catch any bug like issue #165998, which +; impacted the case of epilogues. +; +; Prologue loop guard: Prologue loop is always entered. +; Unrolled loop guard: Unrolled loop is always entered. +; PROLOG: !0 = !{!"branch_weights", i32 1, i32 127} +; +; Prologue loop latch: Prologue loop executes both of its 2 iterations. +; PROLOG: !1 = !{!"branch_weights", i32 0, i32 1} +; +; Unrolled loop latch: Unrolled loop is infinite. +; PROLOG: !4 = !{!"branch_weights", i32 1, i32 0} From 9628a858bfe9306bd7661bb785e9aa89c585ee1c Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" Date: Mon, 3 Nov 2025 18:57:28 -0500 Subject: [PATCH 2/3] Try to improve code comments --- llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp | 12 ++++++------ .../Transforms/LoopUnroll/loop-probability-one.ll | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp b/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp index 1599c743c51cf..99f978997bf8b 100644 --- a/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp +++ b/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp @@ -200,14 +200,14 @@ static void ConnectProlog(Loop *L, Value *BECount, unsigned Count, /// from 0 to \p N more iterations can possibly execute. Among such cases in /// the original loop (with loop probability \p OriginalLoopProb), what is the /// probability of executing at least one more iteration? -/// -/// If \p OriginalLoopProb is 1, then the original loop was somehow determined -/// to be always infinite. Runtime loop unrolling should be impossible for that -/// case. What is infinity % UnrollCount? But we might have bad profile data. -/// In that case, we arbitrarily choose to keep the probability at 1 throughout -/// the remainder loop. static BranchProbability probOfNextInRemainder(BranchProbability OriginalLoopProb, unsigned N) { + // OriginalLoopProb == 1 would produce a division by zero in the calculation + // below. The problem is that case indicates an infinite loop, but runtime + // loop unrolling is not possible for an actual infinite loop as infinity % + // UnrollCount is undefined, so there is no correct result here. We + // arbitrarily choose probabilities indicating that all remainder loop + // iterations will always execute. if (OriginalLoopProb == BranchProbability::getOne()) return BranchProbability::getOne(); diff --git a/llvm/test/Transforms/LoopUnroll/loop-probability-one.ll b/llvm/test/Transforms/LoopUnroll/loop-probability-one.ll index bdd2aaf5b7f4e..845c88ca4e73c 100644 --- a/llvm/test/Transforms/LoopUnroll/loop-probability-one.ll +++ b/llvm/test/Transforms/LoopUnroll/loop-probability-one.ll @@ -2,10 +2,10 @@ ; not crash or otherwise break LoopUnroll behavior when it tries to compute new ; probabilities from it. ; -; Runtime loop unrolling should be impossible for that case. What is infinity % -; UnrollCount? But we can have bad profile data. In that case, the -; implementation arbitrarily chooses to keep the probability at 1 throughout the -; remainder loop. +; That case indicates an infinite loop, but runtime loop unrolling is not +; possible for an actual infinite loop as infinity % UnrollCount is undefined. +; The implementation then arbitrarily chooses probabilities indicating that all +; remainder loop iterations will always execute. ; DEFINE: %{unroll} = opt < %s -unroll-count=3 -passes=loop-unroll -S ; DEFINE: %{rt} = %{unroll} -unroll-runtime From fcc665025410087415c89ce4625aee4e9002ebe5 Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" Date: Tue, 4 Nov 2025 11:25:13 -0500 Subject: [PATCH 3/3] Be even more careful with comments And trigger pre-commit CI again because it was failing on a test that surely this PR did not affect. --- .../Transforms/Utils/LoopUnrollRuntime.cpp | 22 ++++++++++++++----- .../LoopUnroll/loop-probability-one.ll | 10 +++++---- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp b/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp index 99f978997bf8b..6c9467bf4a005 100644 --- a/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp +++ b/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp @@ -203,11 +203,23 @@ static void ConnectProlog(Loop *L, Value *BECount, unsigned Count, static BranchProbability probOfNextInRemainder(BranchProbability OriginalLoopProb, unsigned N) { // OriginalLoopProb == 1 would produce a division by zero in the calculation - // below. The problem is that case indicates an infinite loop, but runtime - // loop unrolling is not possible for an actual infinite loop as infinity % - // UnrollCount is undefined, so there is no correct result here. We - // arbitrarily choose probabilities indicating that all remainder loop - // iterations will always execute. + // below. The problem is that case indicates an always infinite loop, but a + // remainder loop cannot be calculated at run time if the original loop is + // infinite as infinity % UnrollCount is undefined. We then choose + // probabilities indicating that all remainder loop iterations will always + // execute. + // + // Currently, the remainder loop here is an epilogue, which cannot be reached + // if the original loop is infinite, so the aforementioned choice is + // arbitrary. + // + // FIXME: Branch weights still need to be fixed in the case of prologues + // (issue #135812). In that case, the aforementioned choice seems reasonable + // for the goal of maintaining the original loop's block frequencies. That + // is, an infinite loop's initial iterations are not skipped, and the prologue + // loop body might have unique blocks that execute a finite number of times + // if, for example, the original loop body contains conditionals like i < + // UnrollCount. if (OriginalLoopProb == BranchProbability::getOne()) return BranchProbability::getOne(); diff --git a/llvm/test/Transforms/LoopUnroll/loop-probability-one.ll b/llvm/test/Transforms/LoopUnroll/loop-probability-one.ll index 845c88ca4e73c..14f6da42df6b1 100644 --- a/llvm/test/Transforms/LoopUnroll/loop-probability-one.ll +++ b/llvm/test/Transforms/LoopUnroll/loop-probability-one.ll @@ -2,10 +2,12 @@ ; not crash or otherwise break LoopUnroll behavior when it tries to compute new ; probabilities from it. ; -; That case indicates an infinite loop, but runtime loop unrolling is not -; possible for an actual infinite loop as infinity % UnrollCount is undefined. -; The implementation then arbitrarily chooses probabilities indicating that all -; remainder loop iterations will always execute. +; That case indicates an always infinite loop. A remainder loop cannot be +; calculated at run time when the original loop is infinite as infinity % +; UnrollCount is undefined, so consistent remainder loop probabilities are +; difficult or impossible to reason about. The implementation chooses +; probabilities indicating that all remainder loop iterations will always +; execute. ; DEFINE: %{unroll} = opt < %s -unroll-count=3 -passes=loop-unroll -S ; DEFINE: %{rt} = %{unroll} -unroll-runtime