[llvm] [EarlyCSE] Correcting assertion for DSE with invariant loads (PR #141495)
via llvm-commits
llvm-commits at lists.llvm.org
Tue May 27 06:54:02 PDT 2025
https://github.com/zGoldthorpe updated https://github.com/llvm/llvm-project/pull/141495
>From 1cd811beb2367480157e3a46ea711e853833d62f Mon Sep 17 00:00:00 2001
From: Zach Goldthorpe <zgoldtho at ualberta.ca>
Date: Thu, 22 May 2025 17:45:15 -0500
Subject: [PATCH 1/3] Correct EarlyCSE assert for DSE with invariant loads
---
llvm/lib/Transforms/Scalar/EarlyCSE.cpp | 2 +-
.../Transforms/EarlyCSE/invariant-loads.ll | 27 +++++++++++++++++++
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
index 1e378369166c0..211c2d23387ad 100644
--- a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -1704,7 +1704,7 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
// In that case, LastStore should keep its present value since we're
// removing the current store.
assert((!LastStore ||
- ParseMemoryInst(LastStore, TTI).getPointerOperand() ==
+ ParseMemoryInst(LastStore, TTI).getPointerOperand() !=
MemInst.getPointerOperand() ||
MSSA) &&
"can't have an intervening store if not using MemorySSA!");
diff --git a/llvm/test/Transforms/EarlyCSE/invariant-loads.ll b/llvm/test/Transforms/EarlyCSE/invariant-loads.ll
index 88454660711ed..96a8fc3ddc00a 100644
--- a/llvm/test/Transforms/EarlyCSE/invariant-loads.ll
+++ b/llvm/test/Transforms/EarlyCSE/invariant-loads.ll
@@ -4,6 +4,7 @@
; RUN: opt -S -passes='early-cse<memssa>' --enable-knowledge-retention < %s | FileCheck %s --check-prefixes=CHECK,USE_ASSUME
declare void @clobber_and_use(i32)
+declare void @clobber_and_combine(i32, i32)
define void @f_0(ptr %ptr) {
; NO_ASSUME-LABEL: @f_0(
@@ -164,6 +165,32 @@ define void @test_dse1(ptr %p) {
ret void
}
+; Extending @test_dse1, the call can't change the contents of p.invariant.
+; Moreover, the contents of p.invariant cannot change from the store to p.
+define void @test_dse2(ptr noalias %p, ptr noalias %p.invariant) {
+; NO_ASSUME-LABEL: @test_dse2(
+; NO_ASSUME-NEXT: [[V:%.*]] = load i32, ptr [[P:%.*]], align 4
+; NO_ASSUME-NEXT: [[V_INVARIANT:%.*]] = load i32, ptr [[P_INVARIANT:%.*]], align 4, !invariant.load !0
+; NO_ASSUME-NEXT: [[V_COMB:%.*]] = call i32 @clobber_and_combine(i32 [[V]], i32 [[V_INVARIANT]])
+; NO_ASSUME-NEXT: store i32 [[V_COMB]], ptr [[P]], align 4
+; NO_ASSUME-NEXT: ret void
+;
+; USE_ASSUME-LABEL: @test_dse2(
+; USE_ASSUME-NEXT: [[V:%.*]] = load i32, ptr [[P:%.*]], align 4
+; USE_ASSUME-NEXT: [[V_INVARIANT:%.*]] = load i32, ptr [[P_INVARIANT:%.*]], align 4, !invariant.load !0
+; USE_ASSUME-NEXT: [[V_COMB:%.*]] = call i32 @clobber_and_combine(i32 [[V]], i32 [[V_INVARIANT]])
+; USE_ASSUME-NEXT: store i32 [[V_COMB]], ptr [[P]], align 4
+; USE_ASSUME-NEXT: call void @llvm.assume(i1 true) [ "dereferenceable"(ptr [[P_INVARIANT]], i64 4), "nonnull"(ptr [[P_INVARIANT]]), "align"(ptr [[P_INVARIANT]], i64 4) ]
+; USE_ASSUME-NEXT: ret void
+;
+ %v = load i32, ptr %p
+ %v.invariant = load i32, ptr %p.invariant, !invariant.load !{}
+ %v.comb = call i32 @clobber_and_combine(i32 %v, i32 %v.invariant)
+ store i32 %v.comb, ptr %p
+ store i32 %v.invariant, ptr %p.invariant
+ ret void
+}
+
; By assumption, v1 must equal v2 (TODO)
define void @test_false_negative_dse2(ptr %p, i32 %v2) {
; CHECK-LABEL: @test_false_negative_dse2(
>From 7ffee97464574c5134aa07131511ad3554096910 Mon Sep 17 00:00:00 2001
From: Zach Goldthorpe <zgoldtho at ualberta.ca>
Date: Tue, 27 May 2025 08:47:02 -0500
Subject: [PATCH 2/3] Removed assert altogether
---
llvm/lib/Transforms/Scalar/EarlyCSE.cpp | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
index 211c2d23387ad..ee9c83643df2f 100644
--- a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -1699,15 +1699,6 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
LoadValue InVal = AvailableLoads.lookup(MemInst.getPointerOperand());
if (InVal.DefInst &&
InVal.DefInst == getMatchingValue(InVal, MemInst, CurrentGeneration)) {
- // It is okay to have a LastStore to a different pointer here if MemorySSA
- // tells us that the load and store are from the same memory generation.
- // In that case, LastStore should keep its present value since we're
- // removing the current store.
- assert((!LastStore ||
- ParseMemoryInst(LastStore, TTI).getPointerOperand() !=
- MemInst.getPointerOperand() ||
- MSSA) &&
- "can't have an intervening store if not using MemorySSA!");
LLVM_DEBUG(dbgs() << "EarlyCSE DSE (writeback): " << Inst << '\n');
if (!DebugCounter::shouldExecute(CSECounter)) {
LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");
>From 29d1b50d64a5fd3905f2f19a35be620599197ac6 Mon Sep 17 00:00:00 2001
From: Zach Goldthorpe <zgoldtho at ualberta.ca>
Date: Tue, 27 May 2025 08:53:49 -0500
Subject: [PATCH 3/3] formatting
---
llvm/lib/Transforms/Scalar/EarlyCSE.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
index ee9c83643df2f..09cb2f4cb0104 100644
--- a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -1698,7 +1698,8 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
if (MemInst.isValid() && MemInst.isStore()) {
LoadValue InVal = AvailableLoads.lookup(MemInst.getPointerOperand());
if (InVal.DefInst &&
- InVal.DefInst == getMatchingValue(InVal, MemInst, CurrentGeneration)) {
+ InVal.DefInst ==
+ getMatchingValue(InVal, MemInst, CurrentGeneration)) {
LLVM_DEBUG(dbgs() << "EarlyCSE DSE (writeback): " << Inst << '\n');
if (!DebugCounter::shouldExecute(CSECounter)) {
LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");
More information about the llvm-commits
mailing list