AMDGPU: Remove arbitrary SCC liveness scan threshold #94097
Merged
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
When checking if SCC needs to be saved and restored, we need an accurate liveness query. Fixes verifier error from inserting an undef use of SCC. Fixes llvm#92561
|
@llvm/pr-subscribers-backend-amdgpu Author: Matt Arsenault (arsenm) ChangesWhen checking if SCC needs to be saved and restored, we need an accurate liveness query. Fixes verifier error from inserting an undef use of SCC. Fixes #92561 Full diff: https://github.com/llvm/llvm-project/pull/94097.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 779f4c853326d..d8e21da8019a7 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -6437,8 +6437,13 @@ loadMBUFScalarOperandsFromVGPR(const SIInstrInfo &TII, MachineInstr &MI,
// Save SCC. Waterfall Loop may overwrite SCC.
Register SaveSCCReg;
- bool SCCNotDead = (MBB.computeRegisterLiveness(TRI, AMDGPU::SCC, MI, 30) !=
- MachineBasicBlock::LQR_Dead);
+
+ // FIXME: We should maintain SCC liveness while doing the FixSGPRCopies walk
+ // rather than unlimited scan everywhere
+ bool SCCNotDead =
+ MBB.computeRegisterLiveness(TRI, AMDGPU::SCC, MI,
+ std::numeric_limits<unsigned>::max()) !=
+ MachineBasicBlock::LQR_Dead;
if (SCCNotDead) {
SaveSCCReg = MRI.createVirtualRegister(&AMDGPU::SReg_32RegClass);
BuildMI(MBB, Begin, DL, TII.get(AMDGPU::S_CSELECT_B32), SaveSCCReg)
diff --git a/llvm/test/CodeGen/AMDGPU/issue92561-restore-undef-scc-verifier-error.ll b/llvm/test/CodeGen/AMDGPU/issue92561-restore-undef-scc-verifier-error.ll
new file mode 100644
index 0000000000000..0adce2b84aa0d
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/issue92561-restore-undef-scc-verifier-error.ll
@@ -0,0 +1,160 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -global-isel=0 -mtriple=amdgcn -mcpu=gfx1100 -verify-machineinstrs < %s | FileCheck -check-prefix=SDAG %s
+; RUN: llc -global-isel=1 -mtriple=amdgcn -mcpu=gfx1100 -verify-machineinstrs < %s | FileCheck -check-prefix=GISEL %s
+
+; Check for verifier error due to trying to save and restore SCC
+; around a waterfall looop when it was never defined. We have to get
+; an accurate liveness at the use point and cannot rely on an
+; imprecise maybe-live query.
+
+define void @issue92561(ptr addrspace(1) %arg) {
+; SDAG-LABEL: issue92561:
+; SDAG: ; %bb.0: ; %bb
+; SDAG-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; SDAG-NEXT: s_clause 0x1
+; SDAG-NEXT: global_load_b128 v[4:7], v[0:1], off offset:16
+; SDAG-NEXT: global_load_b128 v[0:3], v[0:1], off
+; SDAG-NEXT: v_mov_b32_e32 v8, 0
+; SDAG-NEXT: s_mov_b32 s12, 0
+; SDAG-NEXT: s_mov_b32 s3, exec_lo
+; SDAG-NEXT: s_mov_b32 s13, s12
+; SDAG-NEXT: s_mov_b32 s14, s12
+; SDAG-NEXT: s_mov_b32 s15, s12
+; SDAG-NEXT: s_waitcnt vmcnt(0)
+; SDAG-NEXT: .LBB0_1: ; =>This Inner Loop Header: Depth=1
+; SDAG-NEXT: v_readfirstlane_b32 s4, v0
+; SDAG-NEXT: v_readfirstlane_b32 s5, v1
+; SDAG-NEXT: v_readfirstlane_b32 s6, v2
+; SDAG-NEXT: v_readfirstlane_b32 s7, v3
+; SDAG-NEXT: v_readfirstlane_b32 s8, v4
+; SDAG-NEXT: v_readfirstlane_b32 s9, v5
+; SDAG-NEXT: v_readfirstlane_b32 s10, v6
+; SDAG-NEXT: v_readfirstlane_b32 s11, v7
+; SDAG-NEXT: v_cmp_eq_u64_e32 vcc_lo, s[4:5], v[0:1]
+; SDAG-NEXT: v_cmp_eq_u64_e64 s0, s[6:7], v[2:3]
+; SDAG-NEXT: v_cmp_eq_u64_e64 s1, s[8:9], v[4:5]
+; SDAG-NEXT: s_delay_alu instid0(VALU_DEP_4) | instskip(NEXT) | instid1(VALU_DEP_3)
+; SDAG-NEXT: v_cmp_eq_u64_e64 s2, s[10:11], v[6:7]
+; SDAG-NEXT: s_and_b32 s0, vcc_lo, s0
+; SDAG-NEXT: s_delay_alu instid0(VALU_DEP_2) | instid1(SALU_CYCLE_1)
+; SDAG-NEXT: s_and_b32 s0, s0, s1
+; SDAG-NEXT: s_delay_alu instid0(VALU_DEP_1) | instid1(SALU_CYCLE_1)
+; SDAG-NEXT: s_and_b32 s0, s0, s2
+; SDAG-NEXT: s_delay_alu instid0(SALU_CYCLE_1)
+; SDAG-NEXT: s_and_saveexec_b32 s0, s0
+; SDAG-NEXT: image_sample_c_lz v9, [v8, v8, v8, v8], s[4:11], s[12:15] dmask:0x1 dim:SQ_RSRC_IMG_2D_ARRAY
+; SDAG-NEXT: ; implicit-def: $vgpr0_vgpr1_vgpr2_vgpr3_vgpr4_vgpr5_vgpr6_vgpr7
+; SDAG-NEXT: ; implicit-def: $vgpr8
+; SDAG-NEXT: s_xor_b32 exec_lo, exec_lo, s0
+; SDAG-NEXT: s_cbranch_execnz .LBB0_1
+; SDAG-NEXT: ; %bb.2:
+; SDAG-NEXT: s_mov_b32 exec_lo, s3
+; SDAG-NEXT: v_dual_mov_b32 v0, 0x7fc00000 :: v_dual_mov_b32 v1, 0
+; SDAG-NEXT: v_mov_b32_e32 v2, 1.0
+; SDAG-NEXT: s_mov_b32 s0, s12
+; SDAG-NEXT: s_mov_b32 s1, s12
+; SDAG-NEXT: s_mov_b32 s2, s12
+; SDAG-NEXT: s_mov_b32 s3, s12
+; SDAG-NEXT: s_mov_b32 s4, s12
+; SDAG-NEXT: s_mov_b32 s5, s12
+; SDAG-NEXT: s_mov_b32 s6, s12
+; SDAG-NEXT: s_mov_b32 s7, s12
+; SDAG-NEXT: s_clause 0x2
+; SDAG-NEXT: image_sample_c_lz v0, [v1, v1, v0, v1], s[0:7], s[12:15] dmask:0x1 dim:SQ_RSRC_IMG_2D_ARRAY
+; SDAG-NEXT: image_sample_c_lz v3, [v1, v1, v1, v1], s[0:7], s[12:15] dmask:0x1 dim:SQ_RSRC_IMG_2D_ARRAY
+; SDAG-NEXT: image_sample_c_lz v2, [v1, v2, v1, v1], s[0:7], s[12:15] dmask:0x1 dim:SQ_RSRC_IMG_2D_ARRAY
+; SDAG-NEXT: v_mov_b32_e32 v4, v1
+; SDAG-NEXT: s_waitcnt vmcnt(2)
+; SDAG-NEXT: v_add_f32_e32 v0, v9, v0
+; SDAG-NEXT: s_waitcnt vmcnt(0)
+; SDAG-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(SKIP_1) | instid1(VALU_DEP_2)
+; SDAG-NEXT: v_add_f32_e32 v0, v2, v0
+; SDAG-NEXT: v_mov_b32_e32 v2, v1
+; SDAG-NEXT: v_dual_add_f32 v0, v3, v0 :: v_dual_mov_b32 v3, v1
+; SDAG-NEXT: s_delay_alu instid0(VALU_DEP_1)
+; SDAG-NEXT: v_mul_f32_e32 v0, 0x3e800000, v0
+; SDAG-NEXT: image_store v[0:2], v[3:4], s[0:7] dim:SQ_RSRC_IMG_2D unorm
+; SDAG-NEXT: s_setpc_b64 s[30:31]
+;
+; GISEL-LABEL: issue92561:
+; GISEL: ; %bb.0: ; %bb
+; GISEL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GISEL-NEXT: s_clause 0x1
+; GISEL-NEXT: global_load_b128 v[2:5], v[0:1], off
+; GISEL-NEXT: global_load_b128 v[6:9], v[0:1], off offset:16
+; GISEL-NEXT: v_mov_b32_e32 v0, 0
+; GISEL-NEXT: s_mov_b32 s20, 0
+; GISEL-NEXT: s_mov_b32 s3, exec_lo
+; GISEL-NEXT: s_mov_b32 s21, s20
+; GISEL-NEXT: s_mov_b32 s22, s20
+; GISEL-NEXT: s_mov_b32 s23, s20
+; GISEL-NEXT: s_mov_b32 s4, s20
+; GISEL-NEXT: s_mov_b32 s5, s20
+; GISEL-NEXT: s_mov_b32 s6, s20
+; GISEL-NEXT: s_mov_b32 s7, s20
+; GISEL-NEXT: s_mov_b32 s8, s20
+; GISEL-NEXT: s_mov_b32 s9, s20
+; GISEL-NEXT: s_mov_b32 s10, s20
+; GISEL-NEXT: s_mov_b32 s11, s20
+; GISEL-NEXT: s_waitcnt vmcnt(0)
+; GISEL-NEXT: .LBB0_1: ; =>This Inner Loop Header: Depth=1
+; GISEL-NEXT: v_readfirstlane_b32 s12, v2
+; GISEL-NEXT: v_readfirstlane_b32 s13, v3
+; GISEL-NEXT: v_readfirstlane_b32 s14, v4
+; GISEL-NEXT: v_readfirstlane_b32 s15, v5
+; GISEL-NEXT: v_readfirstlane_b32 s16, v6
+; GISEL-NEXT: v_readfirstlane_b32 s17, v7
+; GISEL-NEXT: v_readfirstlane_b32 s18, v8
+; GISEL-NEXT: v_readfirstlane_b32 s19, v9
+; GISEL-NEXT: v_cmp_eq_u64_e32 vcc_lo, s[12:13], v[2:3]
+; GISEL-NEXT: v_cmp_eq_u64_e64 s0, s[14:15], v[4:5]
+; GISEL-NEXT: v_cmp_eq_u64_e64 s1, s[16:17], v[6:7]
+; GISEL-NEXT: s_delay_alu instid0(VALU_DEP_4) | instskip(NEXT) | instid1(VALU_DEP_3)
+; GISEL-NEXT: v_cmp_eq_u64_e64 s2, s[18:19], v[8:9]
+; GISEL-NEXT: s_and_b32 s0, vcc_lo, s0
+; GISEL-NEXT: s_delay_alu instid0(VALU_DEP_2) | instid1(SALU_CYCLE_1)
+; GISEL-NEXT: s_and_b32 s0, s0, s1
+; GISEL-NEXT: s_delay_alu instid0(VALU_DEP_1) | instid1(SALU_CYCLE_1)
+; GISEL-NEXT: s_and_b32 s0, s0, s2
+; GISEL-NEXT: s_delay_alu instid0(SALU_CYCLE_1)
+; GISEL-NEXT: s_and_saveexec_b32 s0, s0
+; GISEL-NEXT: image_sample_c_lz v1, [v0, v0, v0, v0], s[12:19], s[20:23] dmask:0x1 dim:SQ_RSRC_IMG_2D_ARRAY
+; GISEL-NEXT: ; implicit-def: $vgpr2_vgpr3_vgpr4_vgpr5_vgpr6_vgpr7_vgpr8_vgpr9
+; GISEL-NEXT: ; implicit-def: $vgpr0
+; GISEL-NEXT: s_xor_b32 exec_lo, exec_lo, s0
+; GISEL-NEXT: s_cbranch_execnz .LBB0_1
+; GISEL-NEXT: ; %bb.2:
+; GISEL-NEXT: s_mov_b32 exec_lo, s3
+; GISEL-NEXT: v_dual_mov_b32 v2, 0 :: v_dual_mov_b32 v3, 1.0
+; GISEL-NEXT: v_mov_b32_e32 v0, 0x7fc00000
+; GISEL-NEXT: s_clause 0x2
+; GISEL-NEXT: image_sample_c_lz v0, [v2, v2, v0, v2], s[4:11], s[20:23] dmask:0x1 dim:SQ_RSRC_IMG_2D_ARRAY
+; GISEL-NEXT: image_sample_c_lz v3, [v2, v3, v2, v2], s[4:11], s[20:23] dmask:0x1 dim:SQ_RSRC_IMG_2D_ARRAY
+; GISEL-NEXT: image_sample_c_lz v4, [v2, v2, v2, v2], s[4:11], s[20:23] dmask:0x1 dim:SQ_RSRC_IMG_2D_ARRAY
+; GISEL-NEXT: s_mov_b32 s21, s20
+; GISEL-NEXT: s_waitcnt vmcnt(2)
+; GISEL-NEXT: v_add_f32_e32 v0, v1, v0
+; GISEL-NEXT: s_waitcnt vmcnt(1)
+; GISEL-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(SKIP_1) | instid1(VALU_DEP_1)
+; GISEL-NEXT: v_dual_add_f32 v0, v3, v0 :: v_dual_mov_b32 v3, v2
+; GISEL-NEXT: s_waitcnt vmcnt(0)
+; GISEL-NEXT: v_add_f32_e32 v0, v4, v0
+; GISEL-NEXT: v_dual_mov_b32 v4, s20 :: v_dual_mov_b32 v5, s21
+; GISEL-NEXT: s_delay_alu instid0(VALU_DEP_2)
+; GISEL-NEXT: v_mul_f32_e32 v1, 0x3e800000, v0
+; GISEL-NEXT: image_store v[1:3], v[4:5], s[4:11] dim:SQ_RSRC_IMG_2D unorm
+; GISEL-NEXT: s_setpc_b64 s[30:31]
+bb:
+ %descriptor = load <8 x i32>, ptr addrspace(1) %arg, align 32
+ %needs.waterfall = call float @llvm.amdgcn.image.sample.c.lz.2darray.f32.f32(i32 1, float 0.0, float 0.0, float 0.0, float 0.0, <8 x i32> %descriptor, <4 x i32> zeroinitializer, i1 false, i32 0, i32 0)
+ %i2 = call float @llvm.amdgcn.image.sample.c.lz.2darray.f32.f32(i32 1, float 0.0, float 0.0, float 0x7FF8000000000000, float 0.0, <8 x i32> zeroinitializer, <4 x i32> zeroinitializer, i1 false, i32 0, i32 0)
+ %i3 = fadd float %needs.waterfall, %i2
+ %i4 = call float @llvm.amdgcn.image.sample.c.lz.2darray.f32.f32(i32 1, float 0.0, float 1.000000e+00, float 0.0, float 0.0, <8 x i32> zeroinitializer, <4 x i32> zeroinitializer, i1 false, i32 0, i32 0)
+ %i5 = fadd float %i4, %i3
+ %i6 = call float @llvm.amdgcn.image.sample.c.lz.2darray.f32.f32(i32 1, float 0.0, float 0.0, float 0.0, float 0.0, <8 x i32> zeroinitializer, <4 x i32> zeroinitializer, i1 false, i32 0, i32 0)
+ %i7 = fadd float %i6, %i5
+ %i8 = fmul float %i7, 2.500000e-01
+ %i9 = insertelement <3 x float> zeroinitializer, float %i8, i64 0
+ call void @llvm.amdgcn.image.store.2d.v3f32.i32(<3 x float> %i9, i32 0, i32 0, i32 0, <8 x i32> zeroinitializer, i32 0, i32 0)
+ ret void
+}
|
rampitec
approved these changes
Jun 1, 2024
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.
When checking if SCC needs to be saved and restored, we need an accurate liveness query. Fixes verifier error from inserting an undef use of SCC.
Fixes #92561