[llvm] [InferAddressSpaces] Lower stuck uninitialized values to flat before rewriting (PR #215525)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 11 04:29:51 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Pankaj Dwivedi (PankajDwivedi-25)
<details>
<summary>Changes</summary>
A value in a cycle whose only non-cyclic incoming values are constants that are
not address expressions (e.g. `poison`) never leaves
`UninitializedAddressSpace`, since the join treats uninitialized as the identity.
Inference and rewriting then disagree about it. `updateAddressSpace()` ignores
uninitialized operands, so users of the stuck value are inferred to a specific
address space and cloned. `rewriteWithNewAddressSpaces()` skips uninitialized
values, so the operand never gets a replacement and its `poison` placeholder is
never filled in:
Assertion `NewOp && "poison replacements in ValueWithNewAddrSpace shouldn't
be null"' failed.
Fix: after the fixpoint, lower any value still at `UninitializedAddressSpace` to
`FlatAddrSpace` and propagate again, so its users join to flat and are no longer
rewritten.
Assisted-by: Claude Opus 5
---
Full diff: https://github.com/llvm/llvm-project/pull/215525.diff
2 Files Affected:
- (modified) llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp (+64-25)
- (added) llvm/test/Transforms/InferAddressSpaces/AMDGPU/phi-cycle-uninitialized-addrspace.ll (+151)
``````````diff
diff --git a/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp b/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
index d11fa0e5c9ce0..3820f3e1e45ad 100644
--- a/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
+++ b/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
@@ -229,6 +229,15 @@ class InferAddressSpacesImpl {
ValueToAddrSpaceMapTy &InferredAddrSpace,
PredicatedAddrSpaceMapTy &PredicatedAS) const;
+ // Adds the users of V whose address space may still change to Worklist.
+ void enqueueUsers(Value &V, const ValueToAddrSpaceMapTy &InferredAddrSpace,
+ SetVector<Value *> &Worklist) const;
+
+ // Propagates address spaces out of Worklist until nothing changes.
+ void runToFixPoint(SetVector<Value *> &Worklist,
+ ValueToAddrSpaceMapTy &InferredAddrSpace,
+ PredicatedAddrSpaceMapTy &PredicatedAS) const;
+
// Tries to infer the specific address space of each address expression in
// Postorder.
void inferAddressSpaces(ArrayRef<WeakTrackingVH> Postorder,
@@ -1133,17 +1142,33 @@ bool InferAddressSpacesImpl::run(Function &CurFn) {
PredicatedAS);
}
-// Constants need to be tracked through RAUW to handle cases with nested
-// constant expressions, so wrap values in WeakTrackingVH.
-void InferAddressSpacesImpl::inferAddressSpaces(
- ArrayRef<WeakTrackingVH> Postorder,
- ValueToAddrSpaceMapTy &InferredAddrSpace,
- PredicatedAddrSpaceMapTy &PredicatedAS) const {
- SetVector<Value *> Worklist(llvm::from_range, Postorder);
- // Initially, all expressions are in the uninitialized address space.
- for (Value *V : Postorder)
- InferredAddrSpace[V] = UninitializedAddressSpace;
+void InferAddressSpacesImpl::enqueueUsers(
+ Value &V, const ValueToAddrSpaceMapTy &InferredAddrSpace,
+ SetVector<Value *> &Worklist) const {
+ for (Value *User : V.users()) {
+ // Skip if User is already in the worklist.
+ if (Worklist.count(User))
+ continue;
+
+ ValueToAddrSpaceMapTy::const_iterator Pos = InferredAddrSpace.find(User);
+ // Our algorithm only updates the address spaces of flat address
+ // expressions, which are those in InferredAddrSpace.
+ if (Pos == InferredAddrSpace.end())
+ continue;
+
+ // Function updateAddressSpace moves the address space down a lattice path.
+ // Therefore, nothing to do if User is already inferred as flat (the bottom
+ // element in the lattice).
+ if (Pos->second == FlatAddrSpace)
+ continue;
+
+ Worklist.insert(User);
+ }
+}
+void InferAddressSpacesImpl::runToFixPoint(
+ SetVector<Value *> &Worklist, ValueToAddrSpaceMapTy &InferredAddrSpace,
+ PredicatedAddrSpaceMapTy &PredicatedAS) const {
while (!Worklist.empty()) {
Value *V = Worklist.pop_back_val();
@@ -1152,26 +1177,40 @@ void InferAddressSpacesImpl::inferAddressSpaces(
if (!updateAddressSpace(*V, InferredAddrSpace, PredicatedAS))
continue;
- for (Value *User : V->users()) {
- // Skip if User is already in the worklist.
- if (Worklist.count(User))
- continue;
+ enqueueUsers(*V, InferredAddrSpace, Worklist);
+ }
+}
- auto Pos = InferredAddrSpace.find(User);
- // Our algorithm only updates the address spaces of flat address
- // expressions, which are those in InferredAddrSpace.
- if (Pos == InferredAddrSpace.end())
- continue;
+// Constants need to be tracked through RAUW to handle cases with nested
+// constant expressions, so wrap values in WeakTrackingVH.
+void InferAddressSpacesImpl::inferAddressSpaces(
+ ArrayRef<WeakTrackingVH> Postorder,
+ ValueToAddrSpaceMapTy &InferredAddrSpace,
+ PredicatedAddrSpaceMapTy &PredicatedAS) const {
+ SetVector<Value *> Worklist(llvm::from_range, Postorder);
+ // Initially, all expressions are in the uninitialized address space.
+ for (Value *V : Postorder)
+ InferredAddrSpace[V] = UninitializedAddressSpace;
- // Function updateAddressSpace moves the address space down a lattice
- // path. Therefore, nothing to do if User is already inferred as flat (the
- // bottom element in the lattice).
- if (Pos->second == FlatAddrSpace)
- continue;
+ runToFixPoint(Worklist, InferredAddrSpace, PredicatedAS);
- Worklist.insert(User);
+ // A value still uninitialized here is stuck in a cycle of uninitialized
+ // values and carries no address space information. Lower it to flat so its
+ // users join to flat, instead of being rewritten to reference an operand
+ // that rewriteWithNewAddressSpaces() never converts.
+ SmallVector<Value *, 4> Lowered;
+ for (Value *V : Postorder) {
+ ValueToAddrSpaceMapTy::iterator I = InferredAddrSpace.find(V);
+ if (I->second == UninitializedAddressSpace) {
+ I->second = FlatAddrSpace;
+ Lowered.push_back(V);
}
}
+
+ for (Value *V : Lowered)
+ enqueueUsers(*V, InferredAddrSpace, Worklist);
+
+ runToFixPoint(Worklist, InferredAddrSpace, PredicatedAS);
}
unsigned
diff --git a/llvm/test/Transforms/InferAddressSpaces/AMDGPU/phi-cycle-uninitialized-addrspace.ll b/llvm/test/Transforms/InferAddressSpaces/AMDGPU/phi-cycle-uninitialized-addrspace.ll
new file mode 100644
index 0000000000000..754ce7b976060
--- /dev/null
+++ b/llvm/test/Transforms/InferAddressSpaces/AMDGPU/phi-cycle-uninitialized-addrspace.ll
@@ -0,0 +1,151 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -mtriple=amdgpu9.42-amd-amdhsa -S -passes=infer-address-spaces %s | FileCheck %s
+
+
+; %q is a loop phi whose only non-cyclic incoming value is a constant that is
+; not an address expression, so it stays uninitialized and %p must stay flat.
+define void @phi_cycle_poison_init(i1 %c, ptr addrspace(5) %s) {
+; CHECK-LABEL: define void @phi_cycle_poison_init(
+; CHECK-SAME: i1 [[C:%.*]], ptr addrspace(5) [[S:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: [[CAST:%.*]] = addrspacecast ptr addrspace(5) [[S]] to ptr
+; CHECK-NEXT: br i1 [[C]], label %[[LOOP:.*]], label %[[EXIT:.*]]
+; CHECK: [[LOOP]]:
+; CHECK-NEXT: [[Q:%.*]] = phi ptr [ poison, %[[ENTRY]] ], [ [[Q]], %[[LOOP]] ]
+; CHECK-NEXT: br i1 [[C]], label %[[LOOP]], label %[[EXIT]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: [[P:%.*]] = phi ptr [ [[CAST]], %[[ENTRY]] ], [ [[Q]], %[[LOOP]] ]
+; CHECK-NEXT: store i32 0, ptr [[P]], align 4
+; CHECK-NEXT: ret void
+;
+entry:
+ %cast = addrspacecast ptr addrspace(5) %s to ptr
+ br i1 %c, label %loop, label %exit
+
+loop:
+ %q = phi ptr [ poison, %entry ], [ %q, %loop ]
+ br i1 %c, label %loop, label %exit
+
+exit:
+ %p = phi ptr [ %cast, %entry ], [ %q, %loop ]
+ store i32 0, ptr %p, align 4
+ ret void
+}
+
+; Same, but %q can hold a real flat address, so rewriting the store to
+; addrspace(5) would be a miscompile rather than only a missing replacement.
+define void @phi_cycle_real_flat_addr(i1 %c, ptr addrspace(5) %s) {
+; CHECK-LABEL: define void @phi_cycle_real_flat_addr(
+; CHECK-SAME: i1 [[C:%.*]], ptr addrspace(5) [[S:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: [[CAST:%.*]] = addrspacecast ptr addrspace(5) [[S]] to ptr
+; CHECK-NEXT: br i1 [[C]], label %[[LOOP:.*]], label %[[EXIT:.*]]
+; CHECK: [[LOOP]]:
+; CHECK-NEXT: [[Q:%.*]] = phi ptr [ inttoptr (i64 1234 to ptr), %[[ENTRY]] ], [ [[Q]], %[[LOOP]] ]
+; CHECK-NEXT: br i1 [[C]], label %[[LOOP]], label %[[EXIT]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: [[P:%.*]] = phi ptr [ [[CAST]], %[[ENTRY]] ], [ [[Q]], %[[LOOP]] ]
+; CHECK-NEXT: store i32 0, ptr [[P]], align 4
+; CHECK-NEXT: ret void
+;
+entry:
+ %cast = addrspacecast ptr addrspace(5) %s to ptr
+ br i1 %c, label %loop, label %exit
+
+loop:
+ %q = phi ptr [ inttoptr (i64 1234 to ptr), %entry ], [ %q, %loop ]
+ br i1 %c, label %loop, label %exit
+
+exit:
+ %p = phi ptr [ %cast, %entry ], [ %q, %loop ]
+ store i32 0, ptr %p, align 4
+ ret void
+}
+
+; Same, but the cycle spans two values so it is not trivially foldable.
+define void @phi_gep_cycle(i1 %c, ptr addrspace(5) %s) {
+; CHECK-LABEL: define void @phi_gep_cycle(
+; CHECK-SAME: i1 [[C:%.*]], ptr addrspace(5) [[S:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: [[CAST:%.*]] = addrspacecast ptr addrspace(5) [[S]] to ptr
+; CHECK-NEXT: br i1 [[C]], label %[[LOOP:.*]], label %[[EXIT:.*]]
+; CHECK: [[LOOP]]:
+; CHECK-NEXT: [[Q:%.*]] = phi ptr [ poison, %[[ENTRY]] ], [ [[Q_NEXT:%.*]], %[[LOOP]] ]
+; CHECK-NEXT: [[Q_NEXT]] = getelementptr i8, ptr [[Q]], i64 1
+; CHECK-NEXT: br i1 [[C]], label %[[LOOP]], label %[[EXIT]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: [[P:%.*]] = phi ptr [ [[CAST]], %[[ENTRY]] ], [ [[Q_NEXT]], %[[LOOP]] ]
+; CHECK-NEXT: store i32 0, ptr [[P]], align 4
+; CHECK-NEXT: ret void
+;
+entry:
+ %cast = addrspacecast ptr addrspace(5) %s to ptr
+ br i1 %c, label %loop, label %exit
+
+loop:
+ %q = phi ptr [ poison, %entry ], [ %q.next, %loop ]
+ %q.next = getelementptr i8, ptr %q, i64 1
+ br i1 %c, label %loop, label %exit
+
+exit:
+ %p = phi ptr [ %cast, %entry ], [ %q.next, %loop ]
+ store i32 0, ptr %p, align 4
+ ret void
+}
+
+; The uninitialized value can also come from unreachable code.
+define void @unreachable_phi_cycle(i1 %c, ptr addrspace(5) %s) {
+; CHECK-LABEL: define void @unreachable_phi_cycle(
+; CHECK-SAME: i1 [[C:%.*]], ptr addrspace(5) [[S:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: [[CAST:%.*]] = addrspacecast ptr addrspace(5) [[S]] to ptr
+; CHECK-NEXT: br label %[[EXIT:.*]]
+; CHECK: [[DEAD:.*]]:
+; CHECK-NEXT: [[Q:%.*]] = phi ptr [ [[Q]], %[[DEAD]] ]
+; CHECK-NEXT: br i1 [[C]], label %[[DEAD]], label %[[EXIT]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: [[P:%.*]] = phi ptr [ [[CAST]], %[[ENTRY]] ], [ [[Q]], %[[DEAD]] ]
+; CHECK-NEXT: store i32 0, ptr [[P]], align 4
+; CHECK-NEXT: ret void
+;
+entry:
+ %cast = addrspacecast ptr addrspace(5) %s to ptr
+ br label %exit
+
+dead:
+ %q = phi ptr [ %q, %dead ]
+ br i1 %c, label %dead, label %exit
+
+exit:
+ %p = phi ptr [ %cast, %entry ], [ %q, %dead ]
+ store i32 0, ptr %p, align 4
+ ret void
+}
+
+; A cycle that does have an inferrable incoming value is still rewritten.
+define void @phi_cycle_inferrable(i1 %c, ptr addrspace(5) %s) {
+; CHECK-LABEL: define void @phi_cycle_inferrable(
+; CHECK-SAME: i1 [[C:%.*]], ptr addrspace(5) [[S:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: br label %[[LOOP:.*]]
+; CHECK: [[LOOP]]:
+; CHECK-NEXT: [[Q:%.*]] = phi ptr addrspace(5) [ [[S]], %[[ENTRY]] ], [ [[Q_NEXT:%.*]], %[[LOOP]] ]
+; CHECK-NEXT: [[Q_NEXT]] = getelementptr i8, ptr addrspace(5) [[Q]], i64 1
+; CHECK-NEXT: store i32 0, ptr addrspace(5) [[Q]], align 4
+; CHECK-NEXT: br i1 [[C]], label %[[LOOP]], label %[[EXIT:.*]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: ret void
+;
+entry:
+ %cast = addrspacecast ptr addrspace(5) %s to ptr
+ br label %loop
+
+loop:
+ %q = phi ptr [ %cast, %entry ], [ %q.next, %loop ]
+ %q.next = getelementptr i8, ptr %q, i64 1
+ store i32 0, ptr %q, align 4
+ br i1 %c, label %loop, label %exit
+
+exit:
+ ret void
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/215525
More information about the llvm-commits
mailing list