[llvm] [LoopPeel] Use SmallMapVector for ExitValues to fix nondeterminism. (PR #219384)

Bernhard M. Wiedemann via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 27 23:35:17 PDT 2026


https://github.com/bmwiedemann created https://github.com/llvm/llvm-project/pull/219384

_This 1-line change was proposed by Claude Opus to improve determinism in code generation. I don't know about llvm internals, so I passed it though 2 rounds of reviews and had it add tests that hopefully show relevance._

---- 

When peeling the last iteration, peelLoop() collects the exit block's LCSSA phis and their latch incoming values in ExitValues, and then replaces every one of those phis with the value from the peeled iteration.

ExitValues was a DenseMap keyed on Instruction *, so the replacement order followed the hash of the phi pointers. Where several phis share an incoming value - which is what an LCSSA phi for a value used more than once outside the loop looks like - replaceAllUsesWith() splices the users of each phi onto the same use list, so that order becomes the use list order of the replacement value.

Use lists are not printed by default, but they are what predecessors() and users() walk, so a permuted one reaches later passes and can change their output. This is the same class of bug as 965f9d87adb0, which converted NonLoopBlocksIDom in this function and ExitInfos in LoopUnroll.cpp; ExitValues was missed.

Populating ExitValues from Exit->phis() means a SmallMapVector visits them in the order they appear in the exit block, which is deterministic.

The test failed 200 of 200 runs before this change and passed 200 of 200 after it, both in an ordinary build and in a reverse-iteration one (LLVM_REVERSE_ITERATION=ON). It uses eight LCSSA phis sharing an incoming value because the count decides how reliably it catches the bug: pointers are hashed as (ptr >> 4) ^ (ptr >> 9), so ASLR decides whether they happen to hash into exit block order, and with fewer phis they do so often enough for the test to pass without the fix - four phis failed only 35 of 50 runs of an ordinary build, six failed 46 of 50.

Assisted-by: Claude Opus 5 (Claude Code)

>From f23657a3bf2daf9da7ae5c628422b27f6b9336e7 Mon Sep 17 00:00:00 2001
From: "Bernhard M. Wiedemann" <bwiedemann at suse.de>
Date: Thu, 27 Aug 2026 15:56:41 +0200
Subject: [PATCH] [LoopPeel] Use SmallMapVector for ExitValues to fix
 nondeterminism.

When peeling the last iteration, peelLoop() collects the exit block's
LCSSA phis and their latch incoming values in ExitValues, and then
replaces every one of those phis with the value from the peeled
iteration.

ExitValues was a DenseMap keyed on Instruction *, so the replacement
order followed the hash of the phi pointers. Where several phis share an
incoming value - which is what an LCSSA phi for a value used more than
once outside the loop looks like - replaceAllUsesWith() splices the users
of each phi onto the same use list, so that order becomes the use list
order of the replacement value.

Use lists are not printed by default, but they are what predecessors()
and users() walk, so a permuted one reaches later passes and can change
their output. This is the same class of bug as 965f9d87adb0, which
converted NonLoopBlocksIDom in this function and ExitInfos in
LoopUnroll.cpp; ExitValues was missed.

Populating ExitValues from Exit->phis() means a SmallMapVector visits
them in the order they appear in the exit block, which is deterministic.

The test failed 200 of 200 runs before this change and passed 200 of 200
after it, both in an ordinary build and in a reverse-iteration one
(LLVM_REVERSE_ITERATION=ON). It uses eight LCSSA phis sharing an incoming
value because the count decides how reliably it catches the bug: pointers
are hashed as (ptr >> 4) ^ (ptr >> 9), so ASLR decides whether they
happen to hash into exit block order, and with fewer phis they do so
often enough for the test to pass without the fix - four phis failed only
35 of 50 runs of an ordinary build, six failed 46 of 50.

Assisted-by: Claude Opus 5 (Claude Code)
---
 llvm/lib/Transforms/Utils/LoopPeel.cpp        |  2 +-
 .../peel-last-iteration-nondeterminism.ll     | 53 +++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/Transforms/LoopUnroll/peel-last-iteration-nondeterminism.ll

diff --git a/llvm/lib/Transforms/Utils/LoopPeel.cpp b/llvm/lib/Transforms/Utils/LoopPeel.cpp
index cc45a3e09bd88..b36ab7ba11c22 100644
--- a/llvm/lib/Transforms/Utils/LoopPeel.cpp
+++ b/llvm/lib/Transforms/Utils/LoopPeel.cpp
@@ -1150,7 +1150,7 @@ void llvm::peelLoop(Loop *L, unsigned PeelCount, bool PeelLast, LoopInfo *LI,
   BasicBlock *InsertTop;
   BasicBlock *InsertBot;
   BasicBlock *NewPreHeader = nullptr;
-  DenseMap<Instruction *, Value *> ExitValues;
+  SmallMapVector<Instruction *, Value *, 4> ExitValues;
   if (PeelLast) {
     // It is convenient to split the single exit block from the latch the
     // into 3 parts - two blocks to anchor the peeled copy of the loop body,
diff --git a/llvm/test/Transforms/LoopUnroll/peel-last-iteration-nondeterminism.ll b/llvm/test/Transforms/LoopUnroll/peel-last-iteration-nondeterminism.ll
new file mode 100644
index 0000000000000..324e58c953d1f
--- /dev/null
+++ b/llvm/test/Transforms/LoopUnroll/peel-last-iteration-nondeterminism.ll
@@ -0,0 +1,53 @@
+; RUN: opt -p loop-unroll -unroll-full-max-count=0 -S --preserve-ll-uselistorder < %s | FileCheck %s
+
+; Peeling the last iteration replaces each LCSSA phi in the exit block with the
+; corresponding value from the peeled iteration. When several phis share an
+; incoming value, the order in which they are replaced becomes the use list
+; order of that value, so it must not depend on the iteration order of a
+; pointer-keyed map.
+;
+; The number of phis matters. DenseMapInfo<T *> hashes (ptr >> 4) ^ (ptr >> 9),
+; so with only a few phis the hash order coincides with the exit block order
+; often enough for the test to pass even without the fix: four phis failed 35 of
+; 50 runs of an ordinary build and six failed 46 of 50. The eight used here
+; failed every run.
+
+define void @peel_last_shared_exit_value(i32 %n) {
+; CHECK-LABEL: define void @peel_last_shared_exit_value(
+; CHECK: %sel.peel = select i1 %c.peel, i32 1, i32 2
+; CHECK-NOT: uselistorder i32 %sel.peel
+entry:
+  %sub = add i32 %n, -1
+  br label %loop
+
+loop:
+  %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
+  %c = icmp eq i32 %iv, %sub
+  %sel = select i1 %c, i32 1, i32 2
+  call void @foo(i32 %sel)
+  %iv.next = add i32 %iv, 1
+  %ec = icmp ne i32 %iv.next, %n
+  br i1 %ec, label %loop, label %exit
+
+exit:
+  %p0 = phi i32 [ %sel, %loop ]
+  %p1 = phi i32 [ %sel, %loop ]
+  %p2 = phi i32 [ %sel, %loop ]
+  %p3 = phi i32 [ %sel, %loop ]
+  %p4 = phi i32 [ %sel, %loop ]
+  %p5 = phi i32 [ %sel, %loop ]
+  %p6 = phi i32 [ %sel, %loop ]
+  %p7 = phi i32 [ %sel, %loop ]
+  call void @use(i32 %p0)
+  call void @use(i32 %p1)
+  call void @use(i32 %p2)
+  call void @use(i32 %p3)
+  call void @use(i32 %p4)
+  call void @use(i32 %p5)
+  call void @use(i32 %p6)
+  call void @use(i32 %p7)
+  ret void
+}
+
+declare void @foo(i32)
+declare void @use(i32)



More information about the llvm-commits mailing list