[polly] [Polly] Reject SCoPs with an escaping scalar defined in an empty-doma… (PR #213656)
Shikhar Jain via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 3 05:32:27 PDT 2026
https://github.com/ShikharJ-Corp created https://github.com/llvm/llvm-project/pull/213656
Summary
-------
Polly can generate invalid IR that fails the verifier with "Instruction
does not dominate all uses!" . This patch detects the situation
early and has Polly keep the original loop instead of producing broken
output. Below is what goes wrong and why declining to optimize is
the conservative but correct fix.
Background
----------
Versioning:
Polly optimizes SCoPs. Rather than overwriting a loop, it emits TWO versions
-> an optimized one and the original as a fallback and a runtime check picks
which runs; the two paths rejoin at a block named "polly.merge_new_and_old".
A value computed in the loop but still needed afterwards is an "escaping"
value (e.g. read by a phi just past the region). The merge block is
responsible for forwarding the right version of it. To arrange this, Polly
records a "scalar write" for the value -- a "must-write", i.e. one that
happens whenever the producing statement runs (unlike a conditional
"may-write").
What goes wrong (issue #206551)
-------------------------------
For each statement Polly computes its "domain": the set of iterations in
which it runs. If a statement provably never runs, its domain is empty and
Polly deletes it before codegen (in removeStmtNotInDomainMap).
The bug appears when the ONLY producer of an escaping value is such an
empty-domain statement:
1. Polly deletes the statement, so the optimized version never computes
the value.
2. Polly still builds the merge block to forward it.
3. The merge block then refers to a value that exists only in the
original version.
The result is a value used on a path where it was never defined -- the
definition does not dominate its uses, and the verifier aborts with
"Instruction does not dominate all uses!".
Why we decline to optimize instead of fixing codegen
----------------------------------------------------
Option 1: make the optimized version also produce the escaping value. In
the reduced test case this looks easy, but only by accident -- there the
value is a select that collapses to the constant 0, so storing 0 "works".
In a realistic case the value is not constant: reproducing it would mean
re-creating it and its entire dependency chain inside a statement that, by
definition, never runs.
There is no correct value for it to compute and no machinery to rebuild
that computation in dead code;
Thus any shortcut is UB or simply wrong.
Option 2: keep optimizing but skip versioning. Versioning is not optional
-- executeScopConditionally always emits the split and merge block for any
generated SCoP. "Skipping" it just disables the optimized copy and falls
back to the original loop -- the same outcome as declining, only reached
later after wasted AST/IR generation.
The fix
-------
Inside buildScop, at the point where statement domains are known but the empty statements have not yet been deleted, Polly now checks for the dangerous situation: an escaping value whose must-write lives in a statement that is about to be deleted for having an empty domain. When this is found, Polly rejects the whole SCoP by recording a new assumption (ESCAPINGSCALAR). The region then keeps its original, correct code. This is a local, per-region decision: every other loop in the function is detected and optimized exactly as before.
A regression test is added in
polly/test/CodeGen/broken-dominance-escaping-scalar.ll.
Fixes https://github.com/llvm/llvm-project/issues/206551
>From f6735cc8e0d762df1c21b3ebaf17638c326578c9 Mon Sep 17 00:00:00 2001
From: ShikharJain <shikharj at qti.qualcomm.com>
Date: Mon, 3 Aug 2026 05:05:55 -0700
Subject: [PATCH] [Polly] Reject SCoPs with an escaping scalar defined in an
empty-domain statement
Summary
-------
Polly can generate invalid IR that fails the verifier with "Instruction
does not dominate all uses!" for a specific but real class of loops. This
patch detects that situation early and, when it occurs, has Polly leave
the loop alone (keep the original code) instead of producing broken
output. The rest of this message explains what the situation is, why the
broken IR happens, and why declining to optimize the loop is the only
correct fix rather than trying to patch up code generation.
A little background
-------------------
Polly looks for loop regions it can optimize; each such region is called
a SCoP. When Polly optimizes a SCoP it does not simply overwrite the
original loop. Instead it produces TWO versions of the region:
* an optimized version, and
* the original version, kept as a safe fallback.
At run time a small check (a "runtime condition") decides which version
runs. Afterwards the two paths join back together at a single block that
Polly names "polly.merge_new_and_old". This whole scheme is called
versioning. Its purpose is safety: if some assumption the optimizer made
turns out not to hold at run time, execution can fall back to the
original loop.
Now, some values that are computed inside the loop are still needed after
the loop finishes. Polly calls such a value an "escaping" value. A
typical example is a value computed in the loop body and then read by a
phi node just past the end of the region. Because both the optimized and
the original version might have produced that value, the merge block is
responsible for picking the right one and handing it to whatever comes
after the loop.
To make this work, Polly internally notes that the value has to be
written out so it can be picked up later. In Polly's terms this note is a
"scalar write". It is a "must-write", meaning: whenever the statement
that produces the value runs, this write definitely happens (as opposed
to a "may-write", which only happens on some paths). This detail matters
below.
What goes wrong (issue #206551)
-------------------------------
For every statement in the loop, Polly computes the exact set of
iterations in which that statement runs. This set is called the
statement's "domain". If Polly can prove a statement never actually runs
for any legal set of parameters, its domain is empty, and Polly deletes
that statement before generating code (this happens in
removeStmtNotInDomainMap).
The bug appears when the ONLY place an escaping value is produced is such
a never-executed (empty-domain) statement. Here is the chain of events:
1. Polly deletes the statement because its domain is empty. As a result,
the optimized version of the loop no longer computes the value at
all.
2. Polly still believes the value escapes the loop, so it still builds
the merge block to forward that value onward.
3. The merge block therefore refers to a value that only exists in the
original version and is never produced in the optimized version.
The outcome is IR in which a value is used on a path where it was never
defined. The definition does not dominate all of its uses, and the LLVM
verifier rejects the module with "Instruction does not dominate all
uses!", aborting compilation.
Why we decline to optimize the loop instead of fixing code generation
---------------------------------------------------------------------
Having Polly give up on a loop is a strong step. Polly exists to optimize
loops, so refusing to optimize one should never be done casually. We
looked hard for a narrower fix first, and there genuinely isn't one that
is correct in general. Here is the reasoning.
The obvious idea is: just make the optimized version produce the escaping
value too, so the merge block has something valid to forward. In the
small test case that comes with this patch, that even looks easy, but
only by accident. In that test case the escaping value is a "select"
whose condition is a constant and whose two possible results are both
zero, so the value is really just the constant 0. Storing a 0 on the
optimized side "works" only because the value happened to collapse to a
constant. That is a property of this one reduced example, not of the bug.
Take a realistic case where the escaping value is NOT a constant. To
reproduce it in the optimized version, we would have to re-create the
instruction that computes it AND every instruction feeding into it -- the
entire chain of computations it depends on -- inside the optimized copy.
But recall WHY the statement was deleted in the first place: its domain
is empty, so it never runs. A statement that never runs has no place in
the optimized schedule to put these computations, and there is no correct
value for it to compute. Polly has no machinery to rebuild an arbitrary
chain of computations inside a piece of code that, by definition, does
not execute. Any shortcut -- inventing an undefined value, reading from a
slot that was never written, or borrowing a value from some unrelated
iteration -- is either undefined behavior or simply the wrong answer. In
plain terms: on the optimized path this value has no legitimate value to
give, so there is nothing correct to forward.
A second idea is: keep optimizing the loop but skip the versioning, so
there is no merge block to go wrong. This does not actually help, because
versioning is not an optional feature that can be switched off while still
optimizing. Polly's code generator ALWAYS emits the two-version split and
the merge block for any SCoP it generates code for (the call that builds
them, executeScopConditionally, is made unconditionally). "Turning off
versioning" really means disabling the optimized version entirely and
sending execution back to the original loop -- which is exactly the same
end result as declining to optimize the loop, except reached much later,
after we have already spent time generating an AST and IR that we then
throw away.
So both alternatives collapse to the same conclusion: on the affected
loop there is no valid optimized version we can produce, and the only
correct outcome is to keep the original loop. Doing that early, before
any broken code is generated, is the clean way to get there.
The fix
-------
Inside buildScop, at the point where statement domains are known but the
empty statements have not yet been deleted, Polly now checks for the
dangerous situation: an escaping value whose must-write lives in a
statement that is about to be deleted for having an empty domain. When
this is found, Polly rejects the whole SCoP by recording a new assumption
(ESCAPINGSCALAR). The region then keeps its original, correct code. This
is a local, per-region decision: every other loop in the function is
detected and optimized exactly as before.
A regression test is added in
polly/test/CodeGen/broken-dominance-escaping-scalar.ll.
Fixes https://github.com/llvm/llvm-project/issues/206551
---
polly/include/polly/ScopBuilder.h | 8 +++
polly/include/polly/Support/ScopHelper.h | 1 +
polly/lib/Analysis/ScopBuilder.cpp | 43 ++++++++++++
polly/lib/Analysis/ScopInfo.cpp | 2 +
.../broken-dominance-escaping-scalar.ll | 66 +++++++++++++++++++
5 files changed, 120 insertions(+)
create mode 100644 polly/test/CodeGen/broken-dominance-escaping-scalar.ll
diff --git a/polly/include/polly/ScopBuilder.h b/polly/include/polly/ScopBuilder.h
index a718a5efece62..6c00365d9d639 100644
--- a/polly/include/polly/ScopBuilder.h
+++ b/polly/include/polly/ScopBuilder.h
@@ -243,6 +243,14 @@ class ScopBuilder final {
bool addLoopBoundsToHeaderDomain(
Loop *L, DenseMap<BasicBlock *, isl::set> &InvalidDomainMap);
+ /// Reject SCoPs where an escaping scalar value's only definition lies in a
+ /// statement that will be removed because its domain is empty. Codegen would
+ /// otherwise leave the escaping use referring to a value that does not
+ /// dominate the versioned exit (LLVM issue #206551).
+ ///
+ /// @returns True if the SCoP was rejected (invalidated).
+ bool rejectDoomedEscapingValueWrites();
+
/// Compute the isl representation for the SCEV @p E in this BB.
///
/// @param BB The BB for which isl representation is to be
diff --git a/polly/include/polly/Support/ScopHelper.h b/polly/include/polly/Support/ScopHelper.h
index e5cc6c4fddcf1..48e42ca0d2dfe 100644
--- a/polly/include/polly/Support/ScopHelper.h
+++ b/polly/include/polly/Support/ScopHelper.h
@@ -52,6 +52,7 @@ enum AssumptionKind {
INFINITELOOP,
INVARIANTLOAD,
DELINEARIZATION,
+ ESCAPINGSCALAR,
};
/// Enum to distinguish between assumptions and restrictions.
diff --git a/polly/lib/Analysis/ScopBuilder.cpp b/polly/lib/Analysis/ScopBuilder.cpp
index 762a930dfea6f..fe31521b5e07e 100644
--- a/polly/lib/Analysis/ScopBuilder.cpp
+++ b/polly/lib/Analysis/ScopBuilder.cpp
@@ -3663,6 +3663,38 @@ static void verifyUses(Scop *S, LoopInfo &LI, DominatorTree &DT) {
}
#endif
+bool ScopBuilder::rejectDoomedEscapingValueWrites() {
+ for (ScopStmt &Stmt : scop->Stmts) {
+ BasicBlock *BB = Stmt.getEntryBlock();
+ if (!BB)
+ continue;
+
+ // Mirror removeStmtNotInDomainMap's null-or-empty domain test: a statement
+ // whose entry block has no domain, or an empty domain, will be removed.
+ isl::set Domain = scop->DomainMap.lookup(BB);
+ if (!(Domain.is_null() || Domain.is_empty()))
+ continue;
+
+ // If such a doomed statement is the sole definition of an escaping scalar
+ // value, removing it would leave the escaping use (a PHI in the region
+ // exit under single-exit-edge versioning) referring to a value that does
+ // not dominate the versioned merge block, producing invalid IR
+ // (LLVM issue #206551). Reject the SCoP so the original code is kept.
+ for (MemoryAccess *MA : Stmt) {
+ if (!MA->isValueKind() || !MA->isMustWrite())
+ continue;
+ auto *AccInst = dyn_cast<Instruction>(MA->getAccessValue());
+ if (!AccInst || !scop->contains(AccInst))
+ continue;
+ if (scop->isEscaping(AccInst)) {
+ scop->invalidate(ESCAPINGSCALAR, DebugLoc(), BB);
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
void ScopBuilder::buildScop(Region &R, AssumptionCache &AC) {
scop = Scop::makeScop(R, SE, LI, DT, *SD.getDetectionContext(&R), ORE,
SD.getNextID());
@@ -3745,6 +3777,17 @@ void ScopBuilder::buildScop(Region &R, AssumptionCache &AC) {
Stmt.setInvalidDomain(InvalidDomainMap[getRegionNodeBasicBlock(
Stmt.getRegion()->getNode())]);
+ // Reject SCoPs whose escaping scalar has its sole definition in a
+ // statement that is about to be removed because its domain is empty
+ // (LLVM issue #206551). Doing this here, after domains are known but
+ // before the empty statements are pruned, lets us keep the original code
+ // for the affected region instead of generating invalid IR.
+ if (rejectDoomedEscapingValueWrites()) {
+ POLLY_DEBUG(dbgs() << "Bailing-out: escaping value defined only in "
+ "empty-domain statement\n");
+ return;
+ }
+
// Remove empty statements.
// Exit early in case there are no executable statements left in this scop.
scop->removeStmtNotInDomainMap();
diff --git a/polly/lib/Analysis/ScopInfo.cpp b/polly/lib/Analysis/ScopInfo.cpp
index 38943f2557cf2..2c5d5796c044d 100644
--- a/polly/lib/Analysis/ScopInfo.cpp
+++ b/polly/lib/Analysis/ScopInfo.cpp
@@ -1926,6 +1926,8 @@ static std::string toString(AssumptionKind Kind) {
return "Invariant load";
case DELINEARIZATION:
return "Delinearization";
+ case ESCAPINGSCALAR:
+ return "Escaping scalar";
}
llvm_unreachable("Unknown AssumptionKind!");
}
diff --git a/polly/test/CodeGen/broken-dominance-escaping-scalar.ll b/polly/test/CodeGen/broken-dominance-escaping-scalar.ll
new file mode 100644
index 0000000000000..ec45b6ba6ec2e
--- /dev/null
+++ b/polly/test/CodeGen/broken-dominance-escaping-scalar.ll
@@ -0,0 +1,66 @@
+; RUN: opt %loadNPMPolly '-passes=polly-custom<codegen>' -S %s | FileCheck %s
+;
+; https://github.com/llvm/llvm-project/issues/206551
+;
+; This test runs the Polly code generation pipeline (-passes=polly-custom<codegen>), which detects the
+; loop nest below as a SCoP and, because it uses runtime versioning, would
+; normally emit two copies of the region (an optimized copy and the original
+; copy) guarded by a runtime check and joined at a merge block.
+;
+; The scalar %cond47.us.us.us is defined in block cond.false.us.us.us.1 and
+; is used outside the SCoP by the PHI in for.cond4.preheader.us.us, so it is
+; an "escaping" value. Its defining block, however, is only reachable for
+; parameter values that the SCoP assumes never occur, so that statement's
+; domain is empty and Polly removes it before code generation. In the
+; generated IR the escaping value therefore has no definition on the
+; optimized path, yet the merge-block PHI still refers to it. That value
+; does not dominate the merge block, so the verifier rejects the module with
+; "Instruction does not dominate all uses!" and compilation aborts.
+;
+; The fix detects this situation while the domains are known but before the
+; empty statement is pruned, and rejects the whole SCoP so the original,
+; already-correct code is kept for this region (no versioned copies are
+; generated). This test guards that behavior: Polly must not emit the
+; versioning split/merge blocks for this input.
+;
+; CHECK-NOT: polly.split_new_and_old
+; CHECK-NOT: polly.merge_new_and_old
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @_Z1iiPA4_A4_iiPA4_A4_cS4_S1_(ptr %j, i32 %k, ptr %0) {
+entry:
+ %sext = shl i32 %k, 24
+ %conv10 = ashr i32 %sext, 24
+ %sub11 = add i32 %conv10, -127
+ %wide.trip.count = zext i32 %sub11 to i64
+ br label %for.cond4.preheader.us.us
+
+for.cond4.preheader.us.us: ; preds = %for.cond.cleanup6.us.us, %entry
+ %cond47.us.us116.us = phi i32 [ 0, %entry ], [ %cond47.us.us.us, %for.cond.cleanup6.us.us ]
+ br label %for.body7.us.us.us
+
+for.cond.cleanup6.us.us: ; preds = %for.cond9.for.cond.cleanup13_crit_edge.us.us.us
+ br label %for.cond4.preheader.us.us
+
+for.body7.us.us.us: ; preds = %for.cond9.for.cond.cleanup13_crit_edge.us.us.us, %for.cond4.preheader.us.us
+ br label %for.cond15.preheader.us.us.us
+
+cond.false.us.us.us.1: ; preds = %for.cond15.preheader.us.us.us
+ %cond47.us.us.us = select i1 false, i32 0, i32 0
+ store i32 0, ptr null, align 4
+ %indvars.iv.next = add nsw i64 %indvars.iv, 1
+ %exitcond.not = icmp eq i64 %indvars.iv.next, %wide.trip.count
+ br i1 %exitcond.not, label %for.cond9.for.cond.cleanup13_crit_edge.us.us.us, label %for.cond15.preheader.us.us.us
+
+for.cond15.preheader.us.us.us: ; preds = %cond.false.us.us.us.1, %for.body7.us.us.us
+ %indvars.iv = phi i64 [ %indvars.iv.next, %cond.false.us.us.us.1 ], [ 0, %for.body7.us.us.us ]
+ %cond478486.us.us.us = phi i32 [ 1, %cond.false.us.us.us.1 ], [ 0, %for.body7.us.us.us ]
+ %1 = load i32, ptr %0, align 4
+ store i32 0, ptr %j, align 8
+ br label %cond.false.us.us.us.1
+
+for.cond9.for.cond.cleanup13_crit_edge.us.us.us: ; preds = %cond.false.us.us.us.1
+ br i1 true, label %for.cond.cleanup6.us.us, label %for.body7.us.us.us
+}
More information about the llvm-commits
mailing list