DAG: Use phi in alloca constant case to create virtual registers #130254
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
This is a follow up from 39bf765, for the other case handled here. We would create CopyToReg marked as uniform, even though the end phi would need to use VGPRs due to another divergent input. There's no directly observable change in the final output of the new test, but it does hit this case.
Member
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-llvm-selectiondag Author: Matt Arsenault (arsenm) ChangesThis is a follow up from 39bf765, Full diff: https://github.com/llvm/llvm-project/pull/130254.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index d2df323fce638..c1387528daba7 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -12039,7 +12039,7 @@ SelectionDAGBuilder::HandlePHINodesInSuccessorBlocks(const BasicBlock *LLVMBB) {
assert(isa<AllocaInst>(PHIOp) &&
FuncInfo.StaticAllocaMap.count(cast<AllocaInst>(PHIOp)) &&
"Didn't codegen value into a register!??");
- Reg = FuncInfo.CreateRegs(PHIOp);
+ Reg = FuncInfo.CreateRegs(&PN);
CopyValueToVirtualRegister(PHIOp, Reg);
}
}
diff --git a/llvm/test/CodeGen/AMDGPU/copy-to-reg-frameindex.ll b/llvm/test/CodeGen/AMDGPU/copy-to-reg-frameindex.ll
index 0aea41a190f1e..b5616501900dd 100644
--- a/llvm/test/CodeGen/AMDGPU/copy-to-reg-frameindex.ll
+++ b/llvm/test/CodeGen/AMDGPU/copy-to-reg-frameindex.ll
@@ -35,3 +35,52 @@ done:
store i32 %extract.0, ptr addrspace(1) %out, align 4
ret void
}
+
+; When creating registers for %divergent.alloca.phi, we should report
+; the CopyToReg as divergent values (not uniform just because the
+; alloca is uniform)
+define void @phi_with_alloca_and_divergent_copy_to_reg(ptr addrspace(5) %divergent.private, ptr addrspace(1) %out, i32 %a, i32 %b, i32 %c) {
+; CHECK-LABEL: phi_with_alloca_and_divergent_copy_to_reg:
+; CHECK: ; %bb.0: ; %entry
+; CHECK-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT: v_mov_b32_e32 v7, v2
+; CHECK-NEXT: v_mov_b32_e32 v6, v1
+; CHECK-NEXT: s_mov_b64 s[4:5], 0
+; CHECK-NEXT: v_lshrrev_b32_e64 v2, 6, s32
+; CHECK-NEXT: .LBB1_1: ; %loop
+; CHECK-NEXT: ; =>This Inner Loop Header: Depth=1
+; CHECK-NEXT: v_mov_b32_e32 v1, v2
+; CHECK-NEXT: v_lshl_add_u32 v2, v3, 2, v1
+; CHECK-NEXT: buffer_store_dword v3, v2, s[0:3], 0 offen
+; CHECK-NEXT: v_add_u32_e32 v2, 1, v3
+; CHECK-NEXT: v_cmp_lt_u32_e32 vcc, 15, v2
+; CHECK-NEXT: s_or_b64 s[4:5], vcc, s[4:5]
+; CHECK-NEXT: v_mov_b32_e32 v3, v4
+; CHECK-NEXT: v_mov_b32_e32 v2, v0
+; CHECK-NEXT: s_andn2_b64 exec, exec, s[4:5]
+; CHECK-NEXT: s_cbranch_execnz .LBB1_1
+; CHECK-NEXT: ; %bb.2: ; %done
+; CHECK-NEXT: s_or_b64 exec, exec, s[4:5]
+; CHECK-NEXT: buffer_load_dword v0, v1, s[0:3], 0 offen
+; CHECK-NEXT: s_waitcnt vmcnt(0)
+; CHECK-NEXT: global_store_dword v[6:7], v0, off
+; CHECK-NEXT: s_waitcnt vmcnt(0)
+; CHECK-NEXT: s_setpc_b64 s[30:31]
+entry:
+ %alloca0 = alloca [16 x i32], addrspace(5)
+ br label %loop
+
+loop:
+ %inc = phi i32 [%a, %entry], [%b, %loop]
+ %divergent.alloca.phi = phi ptr addrspace(5) [ %alloca0, %entry ], [ %divergent.private, %loop ]
+ %ptr = getelementptr [16 x i32], ptr addrspace(5) %divergent.alloca.phi, i32 0, i32 %inc
+ store i32 %inc, ptr addrspace(5) %ptr
+ %inc.i = add i32 %inc, 1
+ %cnd = icmp uge i32 %inc.i, 16
+ br i1 %cnd, label %done, label %loop
+
+done:
+ %tmp1 = load i32, ptr addrspace(5) %divergent.alloca.phi
+ store i32 %tmp1, ptr addrspace(1) %out
+ ret void
+}
|
arsenm
requested review from
JanekvO,
JonChesterfield,
Pierre-vh,
cdevadas and
jayfoad
rampitec
approved these changes
Mar 7, 2025
arsenm
deleted the
users/arsenm/dag/use-phi-op-to-create-register-not-constant-case
branch
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/14244 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/33/builds/12642 Here is the relevant piece of the build log for the reference |
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.
This is a follow up from 39bf765,
for the other case handled here. We would create CopyToReg marked
as uniform, even though the end phi would need to use VGPRs due
to another divergent input. There's no directly observable change in
the final output of the new test, but it does hit this case.