[llvm] [AMDGPU] Avoid dangling SSAUpdater reference in PromoteAlloca full-vector store (PR #215686)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 20 15:50:16 PDT 2026
https://github.com/carlobertolli updated https://github.com/llvm/llvm-project/pull/215686
>From 9fa4966e5066e18a91eb7a3d07974f53e90e04c9 Mon Sep 17 00:00:00 2001
From: Carlo Bertolli <carlo.bertolli at amd.com>
Date: Tue, 18 Aug 2026 20:02:48 +0000
Subject: [PATCH 1/5] [AMDGPU] Switch to use a freeze when the value is in the
worklist.
---
.../lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp | 18 ++++++++++---
...promote-alloca-proper-value-replacement.ll | 26 +++++++++++++++++++
2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp b/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
index febb61075d17d..0700772c39770 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
@@ -724,9 +724,21 @@ static Value *promoteAllocaUserToVector(Instruction *Inst, const DataLayout &DL,
// We're storing the full vector, we can handle this without knowing CurVal.
Type *AccessTy = Val->getType();
TypeSize AccessSize = DL.getTypeStoreSize(AccessTy);
- if (Constant *CI = dyn_cast<Constant>(Index))
- if (CI->isNullValue() && AccessSize == VecStoreSize)
- return Builder.CreateBitPreservingCastChain(DL, Val, AA.Vector.Ty);
+ if (Constant *CI = dyn_cast<Constant>(Index)) {
+ if (CI->isNullValue() && AccessSize == VecStoreSize) {
+ Value *Result =
+ Builder.CreateBitPreservingCastChain(DL, Val, AA.Vector.Ty);
+ // If Result is an instruction in the worklist (e.g. a load from this
+ // alloca), it will later be RAUW'd and deleted. The SSAUpdater holds
+ // a raw Value* that RAUW doesn't update, leaving a dangling pointer.
+ // Wrap in a freeze to create a fresh value the SSAUpdater can safely
+ // hold; the freeze's operand is a proper IR use that RAUW does update.
+ if (auto *RI = dyn_cast<Instruction>(Result))
+ if (llvm::is_contained(AA.Vector.Worklist, RI))
+ Result = Builder.CreateFreeze(Result);
+ return Result;
+ }
+ }
// Storing a subvector.
if (isa<FixedVectorType>(AccessTy)) {
diff --git a/llvm/test/CodeGen/AMDGPU/promote-alloca-proper-value-replacement.ll b/llvm/test/CodeGen/AMDGPU/promote-alloca-proper-value-replacement.ll
index 862be6ee61c70..ff289b01c4b3c 100644
--- a/llvm/test/CodeGen/AMDGPU/promote-alloca-proper-value-replacement.ll
+++ b/llvm/test/CodeGen/AMDGPU/promote-alloca-proper-value-replacement.ll
@@ -27,3 +27,29 @@ define void @alloca_value_cross_reference() {
store float 0.000000e+00, ptr addrspace(5) %p, align 4
ret void
}
+
+; The full-vector store in %bb2 forwards %v (a load from the same alloca in a
+; dominating block) as bb2's live-out value. If the pass visits bb2 before
+; entry, it hands the not-yet-replaced load to the SSAUpdater; the load is
+; later deleted while the SSAUpdater still references it, causing a crash.
+; The full-vector store now always creates a fresh value so the SSAUpdater
+; never holds a pointer to a worklist instruction.
+define half @forwarded_load_across_blocks() {
+; CHECK-LABEL: define half @forwarded_load_across_blocks()
+; CHECK-NOT: alloca
+; CHECK-NOT: addrspace(5)
+; CHECK: ret half
+entry:
+ %arr = alloca [4 x half], align 8, addrspace(5)
+ store <4 x half> <half 1.0, half 2.0, half 3.0, half 4.0>, ptr addrspace(5) %arr, align 8
+ %v = load <4 x half>, ptr addrspace(5) %arr, align 8
+ br label %bb2
+
+bb2:
+ store <4 x half> %v, ptr addrspace(5) %arr, align 8
+ br label %bb3
+
+bb3:
+ %e = load half, ptr addrspace(5) %arr, align 2
+ ret half %e
+}
>From bbb180cfbd76d839b45eb4614b584d7e02df983b Mon Sep 17 00:00:00 2001
From: Carlo Bertolli <carlo.bertolli at amd.com>
Date: Wed, 19 Aug 2026 23:39:30 +0000
Subject: [PATCH 2/5] [AMDGPU] Use TrackingVH in SSAUpdater to fix dangling
pointer in promote-alloca
Replace the conditional freeze workaround with a TrackingSSAUpdater wrapper
that keeps a parallel DenseMap<BasicBlock*, TrackingVH<Value>>. TrackingVH
hooks into RAUW via the Value handle mechanism, so when a forwarded load is
replaced the tracked pointer automatically updates to the replacement value.
Before any query that reads the inner SSAUpdater's map, syncAll() pushes the
tracked values back, overwriting any stale raw pointers.
This removes the is_contained check and freeze insertion from the full-vector
store path, addressing review feedback.
Co-authored-by: Cursor <cursoragent at cursor.com>
---
.../lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp | 78 +++++++++++++++----
...promote-alloca-proper-value-replacement.ll | 24 +++---
2 files changed, 76 insertions(+), 26 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp b/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
index 0700772c39770..d22a8169cf1f2 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
@@ -44,6 +44,7 @@
#include "llvm/Pass.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Target/TargetMachine.h"
+#include "llvm/IR/ValueHandle.h"
#include "llvm/Transforms/Utils/SSAUpdater.h"
#define DEBUG_TYPE "amdgpu-promote-alloca"
@@ -52,6 +53,60 @@ using namespace llvm;
namespace {
+/// Wrapper around SSAUpdater that uses TrackingVH to keep available values
+/// up-to-date when the original values are RAUW'd or deleted. The plain
+/// SSAUpdater stores raw Value* pointers that become dangling when a value it
+/// holds is replaced and erased.
+class TrackingSSAUpdater {
+ SSAUpdater Updater;
+ DenseMap<BasicBlock *, TrackingVH<Value>> TrackedVals;
+
+public:
+ explicit TrackingSSAUpdater(
+ SmallVectorImpl<PHINode *> *InsertedPHIs = nullptr)
+ : Updater(InsertedPHIs) {}
+
+ void Initialize(Type *Ty, StringRef Name) {
+ Updater.Initialize(Ty, Name);
+ TrackedVals.clear();
+ }
+
+ void AddAvailableValue(BasicBlock *BB, Value *V) {
+ TrackedVals[BB] = TrackingVH<Value>(V);
+ Updater.AddAvailableValue(BB, V);
+ }
+
+ Value *FindValueForBlock(BasicBlock *BB) const {
+ auto It = TrackedVals.find(BB);
+ if (It == TrackedVals.end())
+ return nullptr;
+ Value *Tracked = It->second;
+ Value *Raw = Updater.FindValueForBlock(BB);
+ if (Raw && Raw != Tracked) {
+ // The tracked value was RAUW'd; update the inner SSAUpdater.
+ const_cast<TrackingSSAUpdater *>(this)->Updater.AddAvailableValue(
+ BB, Tracked);
+ }
+ return Tracked;
+ }
+
+ Value *GetValueInMiddleOfBlock(BasicBlock *BB) {
+ syncAll();
+ return Updater.GetValueInMiddleOfBlock(BB);
+ }
+
+ Value *GetValueAtEndOfBlock(BasicBlock *BB) {
+ syncAll();
+ return Updater.GetValueAtEndOfBlock(BB);
+ }
+
+private:
+ void syncAll() {
+ for (auto &[BB, VH] : TrackedVals)
+ Updater.AddAvailableValue(BB, VH);
+ }
+};
+
static cl::opt<bool>
DisablePromoteAllocaToVector("disable-promote-alloca-to-vector",
cl::desc("Disable promote alloca to vector"),
@@ -724,21 +779,9 @@ static Value *promoteAllocaUserToVector(Instruction *Inst, const DataLayout &DL,
// We're storing the full vector, we can handle this without knowing CurVal.
Type *AccessTy = Val->getType();
TypeSize AccessSize = DL.getTypeStoreSize(AccessTy);
- if (Constant *CI = dyn_cast<Constant>(Index)) {
- if (CI->isNullValue() && AccessSize == VecStoreSize) {
- Value *Result =
- Builder.CreateBitPreservingCastChain(DL, Val, AA.Vector.Ty);
- // If Result is an instruction in the worklist (e.g. a load from this
- // alloca), it will later be RAUW'd and deleted. The SSAUpdater holds
- // a raw Value* that RAUW doesn't update, leaving a dangling pointer.
- // Wrap in a freeze to create a fresh value the SSAUpdater can safely
- // hold; the freeze's operand is a proper IR use that RAUW does update.
- if (auto *RI = dyn_cast<Instruction>(Result))
- if (llvm::is_contained(AA.Vector.Worklist, RI))
- Result = Builder.CreateFreeze(Result);
- return Result;
- }
- }
+ if (Constant *CI = dyn_cast<Constant>(Index))
+ if (CI->isNullValue() && AccessSize == VecStoreSize)
+ return Builder.CreateBitPreservingCastChain(DL, Val, AA.Vector.Ty);
// Storing a subvector.
if (isa<FixedVectorType>(AccessTy)) {
@@ -1128,8 +1171,9 @@ void AMDGPUPromoteAllocaImpl::promoteAllocaToVector(AllocaAnalysis &AA) {
const unsigned ElementSize = DL.getTypeSizeInBits(VecEltTy) / 8;
// Alloca is uninitialized memory. Imitate that by making the first value
- // undef.
- SSAUpdater Updater;
+ // undef. Use TrackingSSAUpdater so that values RAUW'd after being
+ // registered (e.g. a forwarded load) are automatically kept up-to-date.
+ TrackingSSAUpdater Updater;
Updater.Initialize(AA.Vector.Ty, "promotealloca");
BasicBlock *EntryBB = AA.Alloca->getParent();
diff --git a/llvm/test/CodeGen/AMDGPU/promote-alloca-proper-value-replacement.ll b/llvm/test/CodeGen/AMDGPU/promote-alloca-proper-value-replacement.ll
index ff289b01c4b3c..724868b22897c 100644
--- a/llvm/test/CodeGen/AMDGPU/promote-alloca-proper-value-replacement.ll
+++ b/llvm/test/CodeGen/AMDGPU/promote-alloca-proper-value-replacement.ll
@@ -29,16 +29,22 @@ define void @alloca_value_cross_reference() {
}
; The full-vector store in %bb2 forwards %v (a load from the same alloca in a
-; dominating block) as bb2's live-out value. If the pass visits bb2 before
-; entry, it hands the not-yet-replaced load to the SSAUpdater; the load is
-; later deleted while the SSAUpdater still references it, causing a crash.
-; The full-vector store now always creates a fresh value so the SSAUpdater
-; never holds a pointer to a worklist instruction.
+; dominating block) as bb2's live-out value. When the worklist visits the
+; store before the load (they are in different blocks), that operand is the
+; original load instruction, which is later replaced and deleted while the
+; SSAUpdater still references it. TrackingSSAUpdater keeps the reference
+; up-to-date via TrackingVH so the replacement is seen automatically.
define half @forwarded_load_across_blocks() {
-; CHECK-LABEL: define half @forwarded_load_across_blocks()
-; CHECK-NOT: alloca
-; CHECK-NOT: addrspace(5)
-; CHECK: ret half
+; CHECK-LABEL: define half @forwarded_load_across_blocks() {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[ARR:%.*]] = freeze <4 x half> poison
+; CHECK-NEXT: br label %[[BB2:.*]]
+; CHECK: [[BB2]]:
+; CHECK-NEXT: br label %[[BB3:.*]]
+; CHECK: [[BB3]]:
+; CHECK-NEXT: [[TMP0:%.*]] = extractelement <4 x half> <half 1.000000e+00, half 2.000000e+00, half 3.000000e+00, half 4.000000e+00>, i32 0
+; CHECK-NEXT: ret half [[TMP0]]
+;
entry:
%arr = alloca [4 x half], align 8, addrspace(5)
store <4 x half> <half 1.0, half 2.0, half 3.0, half 4.0>, ptr addrspace(5) %arr, align 8
>From 0c396bb1d3a276325af31038c0d9db7477fa3e3a Mon Sep 17 00:00:00 2001
From: Carlo Bertolli <carlo.bertolli at amd.com>
Date: Wed, 19 Aug 2026 23:44:12 +0000
Subject: [PATCH 3/5] [AMDGPU] Fix format.
---
llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp b/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
index d22a8169cf1f2..254e04dd845de 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
@@ -40,11 +40,11 @@
#include "llvm/IR/IntrinsicsAMDGPU.h"
#include "llvm/IR/IntrinsicsR600.h"
#include "llvm/IR/PatternMatch.h"
+#include "llvm/IR/ValueHandle.h"
#include "llvm/InitializePasses.h"
#include "llvm/Pass.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Target/TargetMachine.h"
-#include "llvm/IR/ValueHandle.h"
#include "llvm/Transforms/Utils/SSAUpdater.h"
#define DEBUG_TYPE "amdgpu-promote-alloca"
>From 1564f61bcfcef49ff8d1b71addedf97b05dda6be Mon Sep 17 00:00:00 2001
From: Carlo Bertolli <carlo.bertolli at amd.com>
Date: Thu, 20 Aug 2026 22:41:24 +0000
Subject: [PATCH 4/5] [AMDGPU] Move TrackingSSAUpdater class with SSAUpdater.
---
.../llvm/Transforms/Utils/SSAUpdater.h | 55 ++++++++++++++++++
.../lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp | 57 +------------------
2 files changed, 56 insertions(+), 56 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Utils/SSAUpdater.h b/llvm/include/llvm/Transforms/Utils/SSAUpdater.h
index 63abebc3eed4a..1a3746567bf0c 100644
--- a/llvm/include/llvm/Transforms/Utils/SSAUpdater.h
+++ b/llvm/include/llvm/Transforms/Utils/SSAUpdater.h
@@ -14,7 +14,9 @@
#define LLVM_TRANSFORMS_UTILS_SSAUPDATER_H
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/IR/ValueHandle.h"
#include <string>
namespace llvm {
@@ -138,6 +140,59 @@ class SSAUpdater {
void UpdateDebugValue(Instruction *I, DbgVariableRecord *DbgValue);
};
+/// Wrapper around SSAUpdater that uses TrackingVH to keep available values
+/// up-to-date when the original values are RAUW'd or deleted. The plain
+/// SSAUpdater stores raw Value* pointers that become dangling when a value it
+/// holds is replaced and erased.
+class TrackingSSAUpdater {
+ SSAUpdater Updater;
+ DenseMap<BasicBlock *, TrackingVH<Value>> TrackedVals;
+
+public:
+ explicit TrackingSSAUpdater(
+ SmallVectorImpl<PHINode *> *InsertedPHIs = nullptr)
+ : Updater(InsertedPHIs) {}
+
+ void Initialize(Type *Ty, StringRef Name) {
+ Updater.Initialize(Ty, Name);
+ TrackedVals.clear();
+ }
+
+ void AddAvailableValue(BasicBlock *BB, Value *V) {
+ TrackedVals[BB] = TrackingVH<Value>(V);
+ Updater.AddAvailableValue(BB, V);
+ }
+
+ Value *FindValueForBlock(BasicBlock *BB) const {
+ auto It = TrackedVals.find(BB);
+ if (It == TrackedVals.end())
+ return nullptr;
+ Value *Tracked = It->second;
+ Value *Raw = Updater.FindValueForBlock(BB);
+ if (Raw && Raw != Tracked) {
+ const_cast<TrackingSSAUpdater *>(this)->Updater.AddAvailableValue(
+ BB, Tracked);
+ }
+ return Tracked;
+ }
+
+ Value *GetValueInMiddleOfBlock(BasicBlock *BB) {
+ syncAll();
+ return Updater.GetValueInMiddleOfBlock(BB);
+ }
+
+ Value *GetValueAtEndOfBlock(BasicBlock *BB) {
+ syncAll();
+ return Updater.GetValueAtEndOfBlock(BB);
+ }
+
+private:
+ void syncAll() {
+ for (auto &[BB, VH] : TrackedVals)
+ Updater.AddAvailableValue(BB, VH);
+ }
+};
+
/// Helper class for promoting a collection of loads and stores into SSA
/// Form using the SSAUpdater.
///
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp b/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
index 254e04dd845de..3126353205741 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
@@ -40,7 +40,6 @@
#include "llvm/IR/IntrinsicsAMDGPU.h"
#include "llvm/IR/IntrinsicsR600.h"
#include "llvm/IR/PatternMatch.h"
-#include "llvm/IR/ValueHandle.h"
#include "llvm/InitializePasses.h"
#include "llvm/Pass.h"
#include "llvm/Support/MathExtras.h"
@@ -53,60 +52,6 @@ using namespace llvm;
namespace {
-/// Wrapper around SSAUpdater that uses TrackingVH to keep available values
-/// up-to-date when the original values are RAUW'd or deleted. The plain
-/// SSAUpdater stores raw Value* pointers that become dangling when a value it
-/// holds is replaced and erased.
-class TrackingSSAUpdater {
- SSAUpdater Updater;
- DenseMap<BasicBlock *, TrackingVH<Value>> TrackedVals;
-
-public:
- explicit TrackingSSAUpdater(
- SmallVectorImpl<PHINode *> *InsertedPHIs = nullptr)
- : Updater(InsertedPHIs) {}
-
- void Initialize(Type *Ty, StringRef Name) {
- Updater.Initialize(Ty, Name);
- TrackedVals.clear();
- }
-
- void AddAvailableValue(BasicBlock *BB, Value *V) {
- TrackedVals[BB] = TrackingVH<Value>(V);
- Updater.AddAvailableValue(BB, V);
- }
-
- Value *FindValueForBlock(BasicBlock *BB) const {
- auto It = TrackedVals.find(BB);
- if (It == TrackedVals.end())
- return nullptr;
- Value *Tracked = It->second;
- Value *Raw = Updater.FindValueForBlock(BB);
- if (Raw && Raw != Tracked) {
- // The tracked value was RAUW'd; update the inner SSAUpdater.
- const_cast<TrackingSSAUpdater *>(this)->Updater.AddAvailableValue(
- BB, Tracked);
- }
- return Tracked;
- }
-
- Value *GetValueInMiddleOfBlock(BasicBlock *BB) {
- syncAll();
- return Updater.GetValueInMiddleOfBlock(BB);
- }
-
- Value *GetValueAtEndOfBlock(BasicBlock *BB) {
- syncAll();
- return Updater.GetValueAtEndOfBlock(BB);
- }
-
-private:
- void syncAll() {
- for (auto &[BB, VH] : TrackedVals)
- Updater.AddAvailableValue(BB, VH);
- }
-};
-
static cl::opt<bool>
DisablePromoteAllocaToVector("disable-promote-alloca-to-vector",
cl::desc("Disable promote alloca to vector"),
@@ -1171,7 +1116,7 @@ void AMDGPUPromoteAllocaImpl::promoteAllocaToVector(AllocaAnalysis &AA) {
const unsigned ElementSize = DL.getTypeSizeInBits(VecEltTy) / 8;
// Alloca is uninitialized memory. Imitate that by making the first value
- // undef. Use TrackingSSAUpdater so that values RAUW'd after being
+ // undef. Use TrackingSSAUpdater so that values RAUW'd after being
// registered (e.g. a forwarded load) are automatically kept up-to-date.
TrackingSSAUpdater Updater;
Updater.Initialize(AA.Vector.Ty, "promotealloca");
>From bf3223000bf87f984abf054bd96b63da2c3cbcd0 Mon Sep 17 00:00:00 2001
From: Carlo Bertolli <carlo.bertolli at amd.com>
Date: Thu, 20 Aug 2026 22:49:55 +0000
Subject: [PATCH 5/5] [AMDGPU] Fix comment.
---
llvm/include/llvm/Transforms/Utils/SSAUpdater.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/Transforms/Utils/SSAUpdater.h b/llvm/include/llvm/Transforms/Utils/SSAUpdater.h
index 1a3746567bf0c..3d908975334e5 100644
--- a/llvm/include/llvm/Transforms/Utils/SSAUpdater.h
+++ b/llvm/include/llvm/Transforms/Utils/SSAUpdater.h
@@ -141,7 +141,7 @@ class SSAUpdater {
};
/// Wrapper around SSAUpdater that uses TrackingVH to keep available values
-/// up-to-date when the original values are RAUW'd or deleted. The plain
+/// up-to-date when the original values are RAUW'd or deleted. The plain
/// SSAUpdater stores raw Value* pointers that become dangling when a value it
/// holds is replaced and erased.
class TrackingSSAUpdater {
More information about the llvm-commits
mailing list