[InferAddressSpaces] Extend undef pointer operand support to phi inst #159548
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously undef pointer operand is only supported for select inst, where undef in generic AS behaves like `take the other side`. This PR extends the support to other instructions, e.g. phi inst. Defer joining and inferring constant pointer operand until all other operand AS states considered.
|
@llvm/pr-subscribers-backend-amdgpu Author: Wenju He (wenju-he) ChangesPreviously undef pointer operand is only supported for select inst, where undef in generic AS behaves like This PR extends the support to other instructions, e.g. phi inst. Defer joining and inferring constant pointer operand until all other operand AS states considered. Full diff: https://github.com/llvm/llvm-project/pull/159548.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp b/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
index a097d338a42ca..fffed5c7ce9aa 100644
--- a/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
+++ b/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
@@ -1002,70 +1002,48 @@ bool InferAddressSpacesImpl::updateAddressSpace(
// isAddressExpression should guarantee that V is an operator or an argument.
assert(isa<Operator>(V) || isa<Argument>(V));
- if (isa<Operator>(V) &&
- cast<Operator>(V).getOpcode() == Instruction::Select) {
- const Operator &Op = cast<Operator>(V);
- Value *Src0 = Op.getOperand(1);
- Value *Src1 = Op.getOperand(2);
-
- auto I = InferredAddrSpace.find(Src0);
- unsigned Src0AS = (I != InferredAddrSpace.end())
- ? I->second
- : Src0->getType()->getPointerAddressSpace();
-
- auto J = InferredAddrSpace.find(Src1);
- unsigned Src1AS = (J != InferredAddrSpace.end())
- ? J->second
- : Src1->getType()->getPointerAddressSpace();
-
- auto *C0 = dyn_cast<Constant>(Src0);
- auto *C1 = dyn_cast<Constant>(Src1);
-
- // If one of the inputs is a constant, we may be able to do a constant
- // addrspacecast of it. Defer inferring the address space until the input
- // address space is known.
- if ((C1 && Src0AS == UninitializedAddressSpace) ||
- (C0 && Src1AS == UninitializedAddressSpace))
- return false;
-
- if (C0 && isSafeToCastConstAddrSpace(C0, Src1AS))
- NewAS = Src1AS;
- else if (C1 && isSafeToCastConstAddrSpace(C1, Src0AS))
- NewAS = Src0AS;
- else
- NewAS = joinAddressSpaces(Src0AS, Src1AS);
+ unsigned AS = TTI->getAssumedAddrSpace(&V);
+ if (AS != UninitializedAddressSpace) {
+ // Use the assumed address space directly.
+ NewAS = AS;
} else {
- unsigned AS = TTI->getAssumedAddrSpace(&V);
- if (AS != UninitializedAddressSpace) {
- // Use the assumed address space directly.
- NewAS = AS;
- } else {
- // Otherwise, infer the address space from its pointer operands.
- for (Value *PtrOperand : getPointerOperands(V, *DL, TTI)) {
- auto I = InferredAddrSpace.find(PtrOperand);
- unsigned OperandAS;
- if (I == InferredAddrSpace.end()) {
- OperandAS = PtrOperand->getType()->getPointerAddressSpace();
- if (OperandAS == FlatAddrSpace) {
- // Check AC for assumption dominating V.
- unsigned AS = getPredicatedAddrSpace(*PtrOperand, &V);
- if (AS != UninitializedAddressSpace) {
- LLVM_DEBUG(dbgs()
- << " deduce operand AS from the predicate addrspace "
- << AS << '\n');
- OperandAS = AS;
- // Record this use with the predicated AS.
- PredicatedAS[std::make_pair(&V, PtrOperand)] = OperandAS;
- }
+ // Otherwise, infer the address space from its pointer operands.
+ SmallVector<Constant *, 2> ConstantPtrOps;
+ for (Value *PtrOperand : getPointerOperands(V, *DL, TTI)) {
+ auto I = InferredAddrSpace.find(PtrOperand);
+ unsigned OperandAS;
+ if (I == InferredAddrSpace.end()) {
+ OperandAS = PtrOperand->getType()->getPointerAddressSpace();
+ if (auto *C = dyn_cast<Constant>(PtrOperand);
+ C && OperandAS == FlatAddrSpace) {
+ ConstantPtrOps.push_back(C);
+ continue;
+ }
+ if (OperandAS == FlatAddrSpace) {
+ // Check AC for assumption dominating V.
+ unsigned AS = getPredicatedAddrSpace(*PtrOperand, &V);
+ if (AS != UninitializedAddressSpace) {
+ LLVM_DEBUG(dbgs()
+ << " deduce operand AS from the predicate addrspace "
+ << AS << '\n');
+ OperandAS = AS;
+ // Record this use with the predicated AS.
+ PredicatedAS[std::make_pair(&V, PtrOperand)] = OperandAS;
}
- } else
- OperandAS = I->second;
+ }
+ } else
+ OperandAS = I->second;
- // join(flat, *) = flat. So we can break if NewAS is already flat.
- NewAS = joinAddressSpaces(NewAS, OperandAS);
- if (NewAS == FlatAddrSpace)
- break;
- }
+ // join(flat, *) = flat. So we can break if NewAS is already flat.
+ NewAS = joinAddressSpaces(NewAS, OperandAS);
+ if (NewAS == FlatAddrSpace)
+ break;
+ }
+ if (NewAS != FlatAddrSpace && NewAS != UninitializedAddressSpace) {
+ if (any_of(ConstantPtrOps, [&](Constant *C) {
+ return !isSafeToCastConstAddrSpace(C, NewAS);
+ }))
+ NewAS = FlatAddrSpace;
}
}
diff --git a/llvm/test/Transforms/InferAddressSpaces/AMDGPU/phi-undef.ll b/llvm/test/Transforms/InferAddressSpaces/AMDGPU/phi-undef.ll
new file mode 100644
index 0000000000000..c1fd05ceb8a4e
--- /dev/null
+++ b/llvm/test/Transforms/InferAddressSpaces/AMDGPU/phi-undef.ll
@@ -0,0 +1,36 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -S -passes=infer-address-spaces %s | FileCheck %s
+
+define void @phi_undef(ptr addrspace(1) %arg, <2 x ptr addrspace(1)> %arg1) {
+; CHECK-LABEL: @phi_undef(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TMP0:%.*]] = call i1 @is_leader()
+; CHECK-NEXT: br i1 [[TMP0]], label [[LEADER:%.*]], label [[MERGE:%.*]]
+; CHECK: leader:
+; CHECK-NEXT: br label [[MERGE]]
+; CHECK: merge:
+; CHECK-NEXT: [[I:%.*]] = phi ptr addrspace(1) [ [[ARG:%.*]], [[LEADER]] ], [ undef, [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[I2:%.*]] = phi <2 x ptr addrspace(1)> [ [[ARG1:%.*]], [[LEADER]] ], [ undef, [[ENTRY]] ]
+; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast <2 x ptr addrspace(1)> [[I2]] to <2 x ptr>
+; CHECK-NEXT: [[J:%.*]] = load i8, ptr addrspace(1) [[I]], align 1
+; CHECK-NEXT: [[J1:%.*]] = icmp eq <2 x ptr> [[TMP1]], zeroinitializer
+; CHECK-NEXT: ret void
+;
+entry:
+ %0 = call i1 @is_leader()
+ br i1 %0, label %leader, label %merge
+
+leader:
+ %cast = addrspacecast ptr addrspace(1) %arg to ptr
+ %cast1 = addrspacecast <2 x ptr addrspace(1)> %arg1 to <2 x ptr>
+ br label %merge
+
+merge:
+ %i = phi ptr [%cast, %leader], [undef, %entry]
+ %i1 = phi <2 x ptr> [%cast1, %leader], [undef, %entry]
+ %j = load i8, ptr %i, align 1
+ %j1 = icmp eq <2 x ptr> %i1, zeroinitializer
+ ret void
+}
+
+declare i1 @is_leader()
|
|
@llvm/pr-subscribers-llvm-transforms Author: Wenju He (wenju-he) ChangesPreviously undef pointer operand is only supported for select inst, where undef in generic AS behaves like This PR extends the support to other instructions, e.g. phi inst. Defer joining and inferring constant pointer operand until all other operand AS states considered. Full diff: https://github.com/llvm/llvm-project/pull/159548.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp b/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
index a097d338a42ca..fffed5c7ce9aa 100644
--- a/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
+++ b/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
@@ -1002,70 +1002,48 @@ bool InferAddressSpacesImpl::updateAddressSpace(
// isAddressExpression should guarantee that V is an operator or an argument.
assert(isa<Operator>(V) || isa<Argument>(V));
- if (isa<Operator>(V) &&
- cast<Operator>(V).getOpcode() == Instruction::Select) {
- const Operator &Op = cast<Operator>(V);
- Value *Src0 = Op.getOperand(1);
- Value *Src1 = Op.getOperand(2);
-
- auto I = InferredAddrSpace.find(Src0);
- unsigned Src0AS = (I != InferredAddrSpace.end())
- ? I->second
- : Src0->getType()->getPointerAddressSpace();
-
- auto J = InferredAddrSpace.find(Src1);
- unsigned Src1AS = (J != InferredAddrSpace.end())
- ? J->second
- : Src1->getType()->getPointerAddressSpace();
-
- auto *C0 = dyn_cast<Constant>(Src0);
- auto *C1 = dyn_cast<Constant>(Src1);
-
- // If one of the inputs is a constant, we may be able to do a constant
- // addrspacecast of it. Defer inferring the address space until the input
- // address space is known.
- if ((C1 && Src0AS == UninitializedAddressSpace) ||
- (C0 && Src1AS == UninitializedAddressSpace))
- return false;
-
- if (C0 && isSafeToCastConstAddrSpace(C0, Src1AS))
- NewAS = Src1AS;
- else if (C1 && isSafeToCastConstAddrSpace(C1, Src0AS))
- NewAS = Src0AS;
- else
- NewAS = joinAddressSpaces(Src0AS, Src1AS);
+ unsigned AS = TTI->getAssumedAddrSpace(&V);
+ if (AS != UninitializedAddressSpace) {
+ // Use the assumed address space directly.
+ NewAS = AS;
} else {
- unsigned AS = TTI->getAssumedAddrSpace(&V);
- if (AS != UninitializedAddressSpace) {
- // Use the assumed address space directly.
- NewAS = AS;
- } else {
- // Otherwise, infer the address space from its pointer operands.
- for (Value *PtrOperand : getPointerOperands(V, *DL, TTI)) {
- auto I = InferredAddrSpace.find(PtrOperand);
- unsigned OperandAS;
- if (I == InferredAddrSpace.end()) {
- OperandAS = PtrOperand->getType()->getPointerAddressSpace();
- if (OperandAS == FlatAddrSpace) {
- // Check AC for assumption dominating V.
- unsigned AS = getPredicatedAddrSpace(*PtrOperand, &V);
- if (AS != UninitializedAddressSpace) {
- LLVM_DEBUG(dbgs()
- << " deduce operand AS from the predicate addrspace "
- << AS << '\n');
- OperandAS = AS;
- // Record this use with the predicated AS.
- PredicatedAS[std::make_pair(&V, PtrOperand)] = OperandAS;
- }
+ // Otherwise, infer the address space from its pointer operands.
+ SmallVector<Constant *, 2> ConstantPtrOps;
+ for (Value *PtrOperand : getPointerOperands(V, *DL, TTI)) {
+ auto I = InferredAddrSpace.find(PtrOperand);
+ unsigned OperandAS;
+ if (I == InferredAddrSpace.end()) {
+ OperandAS = PtrOperand->getType()->getPointerAddressSpace();
+ if (auto *C = dyn_cast<Constant>(PtrOperand);
+ C && OperandAS == FlatAddrSpace) {
+ ConstantPtrOps.push_back(C);
+ continue;
+ }
+ if (OperandAS == FlatAddrSpace) {
+ // Check AC for assumption dominating V.
+ unsigned AS = getPredicatedAddrSpace(*PtrOperand, &V);
+ if (AS != UninitializedAddressSpace) {
+ LLVM_DEBUG(dbgs()
+ << " deduce operand AS from the predicate addrspace "
+ << AS << '\n');
+ OperandAS = AS;
+ // Record this use with the predicated AS.
+ PredicatedAS[std::make_pair(&V, PtrOperand)] = OperandAS;
}
- } else
- OperandAS = I->second;
+ }
+ } else
+ OperandAS = I->second;
- // join(flat, *) = flat. So we can break if NewAS is already flat.
- NewAS = joinAddressSpaces(NewAS, OperandAS);
- if (NewAS == FlatAddrSpace)
- break;
- }
+ // join(flat, *) = flat. So we can break if NewAS is already flat.
+ NewAS = joinAddressSpaces(NewAS, OperandAS);
+ if (NewAS == FlatAddrSpace)
+ break;
+ }
+ if (NewAS != FlatAddrSpace && NewAS != UninitializedAddressSpace) {
+ if (any_of(ConstantPtrOps, [&](Constant *C) {
+ return !isSafeToCastConstAddrSpace(C, NewAS);
+ }))
+ NewAS = FlatAddrSpace;
}
}
diff --git a/llvm/test/Transforms/InferAddressSpaces/AMDGPU/phi-undef.ll b/llvm/test/Transforms/InferAddressSpaces/AMDGPU/phi-undef.ll
new file mode 100644
index 0000000000000..c1fd05ceb8a4e
--- /dev/null
+++ b/llvm/test/Transforms/InferAddressSpaces/AMDGPU/phi-undef.ll
@@ -0,0 +1,36 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -S -passes=infer-address-spaces %s | FileCheck %s
+
+define void @phi_undef(ptr addrspace(1) %arg, <2 x ptr addrspace(1)> %arg1) {
+; CHECK-LABEL: @phi_undef(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TMP0:%.*]] = call i1 @is_leader()
+; CHECK-NEXT: br i1 [[TMP0]], label [[LEADER:%.*]], label [[MERGE:%.*]]
+; CHECK: leader:
+; CHECK-NEXT: br label [[MERGE]]
+; CHECK: merge:
+; CHECK-NEXT: [[I:%.*]] = phi ptr addrspace(1) [ [[ARG:%.*]], [[LEADER]] ], [ undef, [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[I2:%.*]] = phi <2 x ptr addrspace(1)> [ [[ARG1:%.*]], [[LEADER]] ], [ undef, [[ENTRY]] ]
+; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast <2 x ptr addrspace(1)> [[I2]] to <2 x ptr>
+; CHECK-NEXT: [[J:%.*]] = load i8, ptr addrspace(1) [[I]], align 1
+; CHECK-NEXT: [[J1:%.*]] = icmp eq <2 x ptr> [[TMP1]], zeroinitializer
+; CHECK-NEXT: ret void
+;
+entry:
+ %0 = call i1 @is_leader()
+ br i1 %0, label %leader, label %merge
+
+leader:
+ %cast = addrspacecast ptr addrspace(1) %arg to ptr
+ %cast1 = addrspacecast <2 x ptr addrspace(1)> %arg1 to <2 x ptr>
+ br label %merge
+
+merge:
+ %i = phi ptr [%cast, %leader], [undef, %entry]
+ %i1 = phi <2 x ptr> [%cast1, %leader], [undef, %entry]
+ %j = load i8, ptr %i, align 1
+ %j1 = icmp eq <2 x ptr> %i1, zeroinitializer
+ ret void
+}
+
+declare i1 @is_leader()
|
There was a problem hiding this comment.
Pull Request Overview
This PR extends the InferAddressSpaces pass to handle undef pointer operands in phi instructions, similar to existing support for select instructions. The change defers joining and inferring constant pointer operand address spaces until all other operand address space states are considered.
- Removes the special-case handling for select instructions only
- Adds generic handling for constant pointer operands in the flat address space across all instruction types
- Defers constant address space casting decisions until non-constant operands are processed
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp | Refactors address space inference to handle constant operands generically instead of select-specific handling |
| llvm/test/Transforms/InferAddressSpaces/AMDGPU/phi-undef.ll | Adds test case verifying phi instructions with undef operands are properly handled |
Comment on lines
+1017
to
+1018
| if (auto *C = dyn_cast<Constant>(PtrOperand); | ||
| C && OperandAS == FlatAddrSpace) { |
There was a problem hiding this comment.
[nitpick] Consider using a more conventional if statement instead of the compound condition in the declaration. The current syntax, while valid, may reduce readability and is less commonly used in LLVM codebase.
Suggested change
| if (auto *C = dyn_cast<Constant>(PtrOperand); | |
| C && OperandAS == FlatAddrSpace) { | |
| auto *C = dyn_cast<Constant>(PtrOperand); | |
| if (C && OperandAS == FlatAddrSpace) { |
| break; | ||
| } | ||
| if (NewAS != FlatAddrSpace && NewAS != UninitializedAddressSpace) { | ||
| if (any_of(ConstantPtrOps, [&](Constant *C) { |
There was a problem hiding this comment.
The lambda capture [&] captures all variables by reference which may be excessive. Consider capturing only the specific variable needed: [NewAS] since that's the only variable used in the lambda.
Suggested change
| if (any_of(ConstantPtrOps, [&](Constant *C) { | |
| if (any_of(ConstantPtrOps, [NewAS](Constant *C) { |
There was a problem hiding this comment.
Agree on not using reference capture
There was a problem hiding this comment.
Agree on not using reference capture
this is also captured:
llvm-project/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:1045:21: error: 'this' cannot be implicitly captured in this context
1045 | return !isSafeToCastConstAddrSpace(C, NewAS);
| ^
llvm-project/llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:1044:40: note: explicitly capture 'this'
1044 | if (any_of(ConstantPtrOps, [NewAS](Constant *C) {
| ^
| , this
|
✅ With the latest revision this PR passed the undef deprecator. |
arsenm
reviewed
Sep 18, 2025
| ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py | ||
| ; RUN: opt -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -S -passes=infer-address-spaces %s | FileCheck %s | ||
|
|
||
| define void @phi_undef(ptr addrspace(1) %arg, <2 x ptr addrspace(1)> %arg1) { |
There was a problem hiding this comment.
Can you also test poison
done
There was a problem hiding this comment.
These can share the same test file
There was a problem hiding this comment.
shall I delete the undef test? pre-commit testing fails with error error: some formatters failed: undef deprecator
There was a problem hiding this comment.
These can share the same test file
merged.
There was a problem hiding this comment.
Yes, I suppose it's not adding that much. Given that we are only rewriting the undefs to a different type, this transform should be undef safe anyway (not entirely sure that is true if we're making use of assume calls)
There was a problem hiding this comment.
Yes, I suppose it's not adding that much. Given that we are only rewriting the undefs to a different type, this transform should be undef safe anyway (not entirely sure that is true if we're making use of assume calls)
done, removed undef test in 9c0780f and keep only poison test.
| ; CHECK-NEXT: ret void | ||
| ; | ||
| entry: | ||
| %0 = call i1 @is_leader() |
There was a problem hiding this comment.
Use named values
done
arsenm
approved these changes
Sep 18, 2025
arsenm
approved these changes
Sep 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously undef pointer operand is only supported for select inst, where undef in generic AS behaves like
take the other side.This PR extends the support to other instructions, e.g. phi inst. Defer joining and inferring constant pointer operand until all other operand AS states considered.