[llvm] [AssumptionCache] Remove incorrect assertion from `removeAffectedValues()` (PR #214524)
Nathan Corbyn via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 10 06:35:27 PDT 2026
https://github.com/cofibrant updated https://github.com/llvm/llvm-project/pull/214524
>From 66dc59f117e18a1c640f769e1f187fa3486fb241 Mon Sep 17 00:00:00 2001
From: Nathan Corbyn <n_corbyn at apple.com>
Date: Thu, 6 Aug 2026 16:51:11 +0100
Subject: [PATCH 1/5] [AssumptionCache] Remove incorrect assertion from
`removeAffectedValues()`
---
llvm/lib/Analysis/AssumptionCache.cpp | 45 ++++---------------
.../remove-from-stale-cache.ll | 40 +++++++++++++++++
2 files changed, 49 insertions(+), 36 deletions(-)
create mode 100644 llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
diff --git a/llvm/lib/Analysis/AssumptionCache.cpp b/llvm/lib/Analysis/AssumptionCache.cpp
index 0794d83a844e2..cf6369ff1f92c 100644
--- a/llvm/lib/Analysis/AssumptionCache.cpp
+++ b/llvm/lib/Analysis/AssumptionCache.cpp
@@ -121,26 +121,6 @@ void AssumptionCache::removeAffectedValues(AssumeInst *CI) {
SmallVector<AssumptionCache::ResultElem, 16> Affected;
findAffectedValues(CI, TTI, Affected);
- // If a value appears more than once in an AssumeInst e.g., 'ptr %arg1' in:
- // call void @llvm.assume(i1 true)
- // [ "dereferenceable"(ptr %arg1, i64 1),
- // "align"(ptr %arg1, i64 8) ]
- // it will appear multiple times in Affected, but we may (depending on
- // how the results in AffectedValues.find_as(AV.Assume) are ordered)
- // nullify multiple instances of Elem.Assume during one iteration of the
- // 'for (auto &AV : Affected)' loop below. The next iteration of that for
- // loop may then find only a match to a different AssumeInst, resulting in
- // an assertion failure. Avoid this by counting the number of expected
- // matches.
-#ifndef NDEBUG
- SmallDenseSet<std::pair<Value *, unsigned>, 16> Seen;
- DenseMap<Value *, int> ExpectedMatches;
- for (auto &AV : Affected)
- if (Seen.insert({AV.Assume, AV.Index}).second &&
- AffectedValues.find_as(AV.Assume) != AffectedValues.end())
- ExpectedMatches[AV.Assume]++;
-#endif
-
for (auto &AV : Affected) {
auto AVI = AffectedValues.find_as(AV.Assume);
if (AVI == AffectedValues.end())
@@ -151,30 +131,23 @@ void AssumptionCache::removeAffectedValues(AssumeInst *CI) {
if (Elem.Assume == CI) {
Found = true;
Elem.Assume = nullptr;
-
-#ifndef NDEBUG
- ExpectedMatches[AV.Assume]--;
-#endif
- assert(ExpectedMatches[AV.Assume] >= 0);
- // After ExpectedMatches[AV.Assume] == 0, we still need to iterate
- // through this loop to determine the value of HasNonnull, to avoid
- // prematurely calling AffectedValues.erase(AVI).
}
+
+ // We need to iterate through this loop to determine the value of
+ // HasNonnull, to avoid prematurely calling AffectedValues.erase(AVI).
HasNonnull |= !!Elem.Assume;
if (HasNonnull && Found)
break;
}
- assert(ExpectedMatches[AV.Assume] == 0 ||
- Found && "already unregistered or incorrect cache state");
-
- if (!HasNonnull)
+ if (!Found) {
+ // It may well be the case that we fail to find an affected value in the
+ // cache. In particular, if an assume call is updated via `Use::set`, we
+ // won't be notified that the affected value has changed and the cache
+ // will silently go stale.
+ } else if (!HasNonnull)
AffectedValues.erase(AVI);
}
-
- assert(
- none_of(Affected, [&](auto &AV) { return ExpectedMatches[AV.Assume]; }) &&
- "already unregistered or incorrect cache state");
}
void AssumptionCache::unregisterAssumption(AssumeInst *CI) {
diff --git a/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll b/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
new file mode 100644
index 0000000000000..95f860438a1a7
--- /dev/null
+++ b/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
@@ -0,0 +1,40 @@
+; RUN: opt < %s -disable-output -passes='no-op-module,loop-rotate,simplifycfg,hotcoldsplit'
+
+; Check that we don't crash on deletion from a stale assumption cache.
+;
+; The no-op module pass initialises the assumption cache, which the SSA updater
+; in `loop-rotate` invalidates it with a call to `Use::set()`. After
+; simplifying the CFG, we're left with a `hotcoldsplit` candidate, leading to a
+; call to `removeAffectedValues()` on a stale cache.
+
+define void @dont_crash(i1 %cond.1) {
+entry:
+ %load = load ptr, ptr null, align 8
+ br i1 %cond.1, label %loop.cond, label %assume
+
+loop.cond:
+ %phi = phi ptr [ null, %loop.body ], [ %load, %entry ]
+ %cond.2 = phi i1 [ false, %loop.body ], [ true, %entry ]
+ br i1 %cond.2, label %loop.body, label %dead
+
+loop.body:
+ call void @llvm.assume(i1 true) [ "nonnull"(ptr %phi) ]
+
+ ; Lengthen code sequence so that `hotcoldsplit` fires (causing the deletion).
+ call void @noise()
+ call void @noise()
+ call void @noise()
+ call void @noise()
+
+ br label %loop.cond
+
+assume:
+ call void @llvm.assume(i1 true) [ "dereferenceable"(ptr %load, i64 8) ]
+ ret void
+
+dead:
+ unreachable
+}
+
+declare void @llvm.assume(i1 noundef)
+declare void @noise()
>From 69e5cad8515c2725a823429456f05670ee800342 Mon Sep 17 00:00:00 2001
From: Nathan Corbyn <n_corbyn at apple.com>
Date: Thu, 6 Aug 2026 17:43:22 +0100
Subject: [PATCH 2/5] Fix typos
---
llvm/lib/Analysis/AssumptionCache.cpp | 2 +-
llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Analysis/AssumptionCache.cpp b/llvm/lib/Analysis/AssumptionCache.cpp
index cf6369ff1f92c..87e63e95a46d6 100644
--- a/llvm/lib/Analysis/AssumptionCache.cpp
+++ b/llvm/lib/Analysis/AssumptionCache.cpp
@@ -142,7 +142,7 @@ void AssumptionCache::removeAffectedValues(AssumeInst *CI) {
if (!Found) {
// It may well be the case that we fail to find an affected value in the
- // cache. In particular, if an assume call is updated via `Use::set`, we
+ // cache. In particular, if an assume call is updated via `Use::set()`, we
// won't be notified that the affected value has changed and the cache
// will silently go stale.
} else if (!HasNonnull)
diff --git a/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll b/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
index 95f860438a1a7..195ee3ce45c86 100644
--- a/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
+++ b/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
@@ -2,7 +2,7 @@
; Check that we don't crash on deletion from a stale assumption cache.
;
-; The no-op module pass initialises the assumption cache, which the SSA updater
+; The no-op module pass initialises the assumption cache, while the SSA updater
; in `loop-rotate` invalidates it with a call to `Use::set()`. After
; simplifying the CFG, we're left with a `hotcoldsplit` candidate, leading to a
; call to `removeAffectedValues()` on a stale cache.
>From 03ff1da24193405341a4a62fcb461538725e6b3c Mon Sep 17 00:00:00 2001
From: Nathan Corbyn <n_corbyn at apple.com>
Date: Thu, 6 Aug 2026 18:05:29 +0100
Subject: [PATCH 3/5] Remove UB from test
---
.../Analysis/AssumptionCache/remove-from-stale-cache.ll | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll b/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
index 195ee3ce45c86..5bad4ab118294 100644
--- a/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
+++ b/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
@@ -7,13 +7,12 @@
; simplifying the CFG, we're left with a `hotcoldsplit` candidate, leading to a
; call to `removeAffectedValues()` on a stale cache.
-define void @dont_crash(i1 %cond.1) {
+define void @dont_crash(i1 %cond.1, ptr %ptr) {
entry:
- %load = load ptr, ptr null, align 8
br i1 %cond.1, label %loop.cond, label %assume
loop.cond:
- %phi = phi ptr [ null, %loop.body ], [ %load, %entry ]
+ %phi = phi ptr [ null, %loop.body ], [ %ptr, %entry ]
%cond.2 = phi i1 [ false, %loop.body ], [ true, %entry ]
br i1 %cond.2, label %loop.body, label %dead
@@ -29,7 +28,7 @@ loop.body:
br label %loop.cond
assume:
- call void @llvm.assume(i1 true) [ "dereferenceable"(ptr %load, i64 8) ]
+ call void @llvm.assume(i1 true) [ "dereferenceable"(ptr %ptr, i64 8) ]
ret void
dead:
>From c1478eb065077285c8e26203a91244fd857429a8 Mon Sep 17 00:00:00 2001
From: Nathan Corbyn <n_corbyn at apple.com>
Date: Mon, 10 Aug 2026 14:12:51 +0100
Subject: [PATCH 4/5] Add assumption cache output to test
---
.../Analysis/AssumptionCache/remove-from-stale-cache.ll | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll b/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
index 5bad4ab118294..b09b56e006b55 100644
--- a/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
+++ b/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
@@ -1,13 +1,16 @@
-; RUN: opt < %s -disable-output -passes='no-op-module,loop-rotate,simplifycfg,hotcoldsplit'
+; RUN: opt < %s -disable-output -passes='module(print<assumptions>),loop-rotate,simplifycfg,hotcoldsplit' 2>&1 | FileCheck %s
; Check that we don't crash on deletion from a stale assumption cache.
;
-; The no-op module pass initialises the assumption cache, while the SSA updater
-; in `loop-rotate` invalidates it with a call to `Use::set()`. After
+; The initial module pass initialises the assumption cache, while the SSA
+; updater in `loop-rotate` invalidates it with a call to `Use::set()`. After
; simplifying the CFG, we're left with a `hotcoldsplit` candidate, leading to a
; call to `removeAffectedValues()` on a stale cache.
define void @dont_crash(i1 %cond.1, ptr %ptr) {
+; CHECK-LABEL: Cached assumptions for function: dont_crash
+; CHECK-NEXT: [ "nonnull"(ptr %phi) ]
+; CHECK-NEXT: [ "dereferenceable"(ptr %ptr, i64 8) ]
entry:
br i1 %cond.1, label %loop.cond, label %assume
>From 80f4e0bb0b0b8ab366dd5664f9ea8e25004a5181 Mon Sep 17 00:00:00 2001
From: Nathan Corbyn <n_corbyn at apple.com>
Date: Mon, 10 Aug 2026 14:35:11 +0100
Subject: [PATCH 5/5] Move assumption printing to after opts
---
llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll b/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
index b09b56e006b55..5ec7c994d6d90 100644
--- a/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
+++ b/llvm/test/Analysis/AssumptionCache/remove-from-stale-cache.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -disable-output -passes='module(print<assumptions>),loop-rotate,simplifycfg,hotcoldsplit' 2>&1 | FileCheck %s
+; RUN: opt < %s -disable-output -passes='no-op-module,loop-rotate,simplifycfg,print<assumptions>,hotcoldsplit' 2>&1 | FileCheck %s
; Check that we don't crash on deletion from a stale assumption cache.
;
@@ -9,7 +9,7 @@
define void @dont_crash(i1 %cond.1, ptr %ptr) {
; CHECK-LABEL: Cached assumptions for function: dont_crash
-; CHECK-NEXT: [ "nonnull"(ptr %phi) ]
+; CHECK-NEXT: [ "nonnull"(ptr %ptr) ]
; CHECK-NEXT: [ "dereferenceable"(ptr %ptr, i64 8) ]
entry:
br i1 %cond.1, label %loop.cond, label %assume
More information about the llvm-commits
mailing list