[AMDGPU] Make AllocaInst return AS5 in getAssumedAddrSpace #136798
Closed
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
Contributor Author
| Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Member
| @llvm/pr-subscribers-backend-amdgpu Author: Shilei Tian (shiltian) ChangesFull diff: https://github.com/llvm/llvm-project/pull/136798.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp index b6cc5137d711a..2c4052a30b10f 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp @@ -951,6 +951,9 @@ bool AMDGPUTargetMachine::isNoopAddrSpaceCast(unsigned SrcAS, } unsigned AMDGPUTargetMachine::getAssumedAddrSpace(const Value *V) const { + if (isa<AllocaInst>(V)) + return AMDGPUAS::PRIVATE_ADDRESS; + const auto *LD = dyn_cast<LoadInst>(V); if (!LD) // TODO: Handle invariant load like constant. return AMDGPUAS::UNKNOWN_ADDRESS_SPACE; diff --git a/llvm/test/Transforms/InferAddressSpaces/AMDGPU/alloca-as0.ll b/llvm/test/Transforms/InferAddressSpaces/AMDGPU/alloca-as0.ll new file mode 100644 index 0000000000000..57dcd96594893 --- /dev/null +++ b/llvm/test/Transforms/InferAddressSpaces/AMDGPU/alloca-as0.ll @@ -0,0 +1,35 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 +; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -passes=infer-address-spaces %s -o - | FileCheck %s + +declare void @bar(ptr) + +define i32 @static_alloca() { +; CHECK-LABEL: define i32 @static_alloca() { +; CHECK-NEXT: [[ALLOCA:%.*]] = alloca i32, align 4 +; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast ptr [[ALLOCA]] to ptr addrspace(5) +; CHECK-NEXT: [[TMP2:%.*]] = addrspacecast ptr addrspace(5) [[TMP1]] to ptr +; CHECK-NEXT: call void @bar(ptr [[TMP2]]) +; CHECK-NEXT: [[LOAD:%.*]] = load i32, ptr addrspace(5) [[TMP1]], align 4 +; CHECK-NEXT: ret i32 [[LOAD]] +; + %alloca = alloca i32, align 4 + call void @bar(ptr %alloca) + %load = load i32, ptr %alloca + ret i32 %load +} + +define i32 @dynamic_alloca(i32 %n) { +; CHECK-LABEL: define i32 @dynamic_alloca( +; CHECK-SAME: i32 [[N:%.*]]) { +; CHECK-NEXT: [[ALLOCA:%.*]] = alloca i32, i32 [[N]], align 4 +; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast ptr [[ALLOCA]] to ptr addrspace(5) +; CHECK-NEXT: [[TMP2:%.*]] = addrspacecast ptr addrspace(5) [[TMP1]] to ptr +; CHECK-NEXT: call void @bar(ptr [[TMP2]]) +; CHECK-NEXT: [[LOAD:%.*]] = load i32, ptr addrspace(5) [[TMP1]], align 4 +; CHECK-NEXT: ret i32 0 +; + %alloca = alloca i32, i32 %n, align 4 + call void @bar(ptr %alloca) + %load = load i32, ptr %alloca + ret i32 0 +} |
Member
| @llvm/pr-subscribers-llvm-transforms Author: Shilei Tian (shiltian) ChangesFull diff: https://github.com/llvm/llvm-project/pull/136798.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp index b6cc5137d711a..2c4052a30b10f 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp @@ -951,6 +951,9 @@ bool AMDGPUTargetMachine::isNoopAddrSpaceCast(unsigned SrcAS, } unsigned AMDGPUTargetMachine::getAssumedAddrSpace(const Value *V) const { + if (isa<AllocaInst>(V)) + return AMDGPUAS::PRIVATE_ADDRESS; + const auto *LD = dyn_cast<LoadInst>(V); if (!LD) // TODO: Handle invariant load like constant. return AMDGPUAS::UNKNOWN_ADDRESS_SPACE; diff --git a/llvm/test/Transforms/InferAddressSpaces/AMDGPU/alloca-as0.ll b/llvm/test/Transforms/InferAddressSpaces/AMDGPU/alloca-as0.ll new file mode 100644 index 0000000000000..57dcd96594893 --- /dev/null +++ b/llvm/test/Transforms/InferAddressSpaces/AMDGPU/alloca-as0.ll @@ -0,0 +1,35 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 +; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -passes=infer-address-spaces %s -o - | FileCheck %s + +declare void @bar(ptr) + +define i32 @static_alloca() { +; CHECK-LABEL: define i32 @static_alloca() { +; CHECK-NEXT: [[ALLOCA:%.*]] = alloca i32, align 4 +; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast ptr [[ALLOCA]] to ptr addrspace(5) +; CHECK-NEXT: [[TMP2:%.*]] = addrspacecast ptr addrspace(5) [[TMP1]] to ptr +; CHECK-NEXT: call void @bar(ptr [[TMP2]]) +; CHECK-NEXT: [[LOAD:%.*]] = load i32, ptr addrspace(5) [[TMP1]], align 4 +; CHECK-NEXT: ret i32 [[LOAD]] +; + %alloca = alloca i32, align 4 + call void @bar(ptr %alloca) + %load = load i32, ptr %alloca + ret i32 %load +} + +define i32 @dynamic_alloca(i32 %n) { +; CHECK-LABEL: define i32 @dynamic_alloca( +; CHECK-SAME: i32 [[N:%.*]]) { +; CHECK-NEXT: [[ALLOCA:%.*]] = alloca i32, i32 [[N]], align 4 +; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast ptr [[ALLOCA]] to ptr addrspace(5) +; CHECK-NEXT: [[TMP2:%.*]] = addrspacecast ptr addrspace(5) [[TMP1]] to ptr +; CHECK-NEXT: call void @bar(ptr [[TMP2]]) +; CHECK-NEXT: [[LOAD:%.*]] = load i32, ptr addrspace(5) [[TMP1]], align 4 +; CHECK-NEXT: ret i32 0 +; + %alloca = alloca i32, i32 %n, align 4 + call void @bar(ptr %alloca) + %load = load i32, ptr %alloca + ret i32 0 +} |
shiltian force-pushed the users/shiltian/alloca-as0-inferas branch from fb9ba07 to b5f7d3d Compare shiltian force-pushed the users/shiltian/amdgpu-alloca-as0 branch from ae09399 to c77ed29 Compare 
shiltian commented Apr 23, 2025
| ; GI-NEXT: v_mov_b32_e32 v1, s15 | ||
| ; GI-NEXT: s_mov_b32 s14, s16 | ||
| ; GI-NEXT: s_movk_i32 s32, 0x400 | ||
| ; GI-NEXT: s_mov_b32 s36, 0 |
Contributor Author
There was a problem hiding this comment.
That's why I explicitly added -O0 in the test case before.
arsenm reviewed Apr 23, 2025
| } | ||
| | ||
| unsigned AMDGPUTargetMachine::getAssumedAddrSpace(const Value *V) const { | ||
| if (isa<AllocaInst>(V)) |
Contributor
There was a problem hiding this comment.
Is this only reachable for flat typed values?
Contributor Author
There was a problem hiding this comment.
I don't think we need to differentiate them. It is either flat or private. It can't be something else, especially after #135820.
Comment on lines +8 to +10
| ; CHECK-NEXT: [[ALLOCA:%.*]] = alloca i32, align 4 | ||
| ; CHECK-NEXT: [[TMP1:%.*]] = addrspacecast ptr [[ALLOCA]] to ptr addrspace(5) | ||
| ; CHECK-NEXT: [[TMP2:%.*]] = addrspacecast ptr addrspace(5) [[TMP1]] to ptr |
Contributor
There was a problem hiding this comment.
We probably don't manage to eliminate this cast pair...
Contributor Author
There was a problem hiding this comment.
No, this pair is for the function call. Not for the load.
By "eliminate this cast pair", do you mean to rewrite alloca with the right AS, or to use the original alloca directly for the function call?
Contributor
There was a problem hiding this comment.
I didn't say anything about a load. I don't think this pair is legally folded out by instcombine
Contributor Author
There was a problem hiding this comment.
Oh we don't run instcombine here. Let me add it.
Contributor
There was a problem hiding this comment.
No, shouldn't run instcombine here. It's unnecessary, we should see the raw pass output. It's a question of whether the full pass pipeline eliminates the cast. There was a recent DAG combine where IIRC the conclusion was instcombine folds these but it's not necessarily legal
Contributor Author
There was a problem hiding this comment.
K, does a follow-up work?
shiltian force-pushed the users/shiltian/amdgpu-alloca-as0 branch from c77ed29 to d04b738 Compare shiltian force-pushed the users/shiltian/alloca-as0-inferas branch from b5f7d3d to 8b975d2 Compare shiltian force-pushed the users/shiltian/amdgpu-alloca-as0 branch from d04b738 to 1f1cbe2 Compare shiltian force-pushed the users/shiltian/alloca-as0-inferas branch from 8b975d2 to 9d2612c Compare 
shiltian force-pushed the users/shiltian/alloca-as0-inferas branch from 9d2612c to 2d75ec2 Compare shiltian force-pushed the users/shiltian/amdgpu-alloca-as0 branch from 1f1cbe2 to 01e6991 Compare Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
No description provided.