[llvm] [AMDGPU] Fix invalid 64-bit DPP formed from folding a 32-bit DPP mov (PR #222216)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-amdgpu

Author: Domenic Nutile (saxlungs)

<details>
<summary>Changes</summary>

GCNDPPCombine already rejects 64-bit DPP when the DPP move itself is
64-bit, but a 32-bit move folded into a source of a 64-bit instruction
was not checked. On targets without FeatureDPALU_DPP this formed
V_LSHLREV_B64_e64_dpp, whose tied old operand got the move's 32-bit
register class instead of the 64-bit destination class.

---

<sub>Stack created with <a href="https://github.com/github/gh-stack">GitHub Stacks CLI</a> • <a href="https://gh.io/stacks-feedback">Give Feedback 💬</a></sub>

---
Full diff: https://github.com/llvm/llvm-project/pull/222216.diff


3 Files Affected:

- (modified) llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp (+5-1) 
- (modified) llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h (+1-1) 
- (modified) llvm/test/CodeGen/AMDGPU/dpp_combine_rev_opcode.ll (+10-6) 


``````````diff
diff --git a/llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp b/llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
index bcfccd98ada0a..2f0e8c61ffcbd 100644
--- a/llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
@@ -757,8 +757,12 @@ bool GCNDPPCombine::combineDPPMov(MachineInstr &MovMI) const {
       break;
     }
 
+    // Without DPALU DPP there are no 64-bit DPP encodings. The 64-bit move is
+    // rejected above, but a 32-bit move folded into a source of a 64-bit
+    // instruction reaches here, so the operands have to be checked too.
     if (!ST->hasFeature(AMDGPU::FeatureDPALU_DPP) &&
-        AMDGPU::isDPALU_DPP32BitOpc(OrigOp)) {
+        (AMDGPU::isDPALU_DPP32BitOpc(OrigOp) ||
+         AMDGPU::hasAny64BitVGPROperands(TII->get(OrigOp), *TII, *ST))) {
       LLVM_DEBUG(dbgs() << "  " << OrigMI
                         << "  failed: DPP ALU DPP is not supported\n");
       break;
diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
index 664ca91e4e7d4..d9accf361c109 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
@@ -1735,7 +1735,7 @@ inline bool isLegalDPALU_DPPControl(const MCSubtargetInfo &ST, unsigned DC) {
 }
 
 /// \returns true if an instruction may have a 64-bit VGPR operand.
-bool hasAny64BitVGPROperands(const MCInstrDesc &OpDesc,
+bool hasAny64BitVGPROperands(const MCInstrDesc &OpDesc, const MCInstrInfo &MII,
                              const MCSubtargetInfo &ST);
 
 /// \returns true if an instruction is a DP ALU DPP without any 64-bit operands.
diff --git a/llvm/test/CodeGen/AMDGPU/dpp_combine_rev_opcode.ll b/llvm/test/CodeGen/AMDGPU/dpp_combine_rev_opcode.ll
index d414f265ad8a8..2ce1b3c55e9ae 100644
--- a/llvm/test/CodeGen/AMDGPU/dpp_combine_rev_opcode.ll
+++ b/llvm/test/CodeGen/AMDGPU/dpp_combine_rev_opcode.ll
@@ -1273,14 +1273,17 @@ define amdgpu_kernel void @dpp_shl_i64_amount(ptr addrspace(1) %arg, i32 %in) {
 ;
 ; GFX11-LABEL: dpp_shl_i64_amount:
 ; GFX11:       ; %bb.0:
+; GFX11-NEXT:    s_clause 0x1
 ; GFX11-NEXT:    s_load_b64 s[0:1], s[4:5], 0x24
+; GFX11-NEXT:    s_load_b32 s2, s[4:5], 0x2c
 ; GFX11-NEXT:    v_and_b32_e32 v2, 0x3ff, v0
-; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_1)
-; GFX11-NEXT:    v_lshlrev_b32_e32 v3, 3, v2
 ; GFX11-NEXT:    s_waitcnt lgkmcnt(0)
+; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_1) | instskip(SKIP_3) | instid1(VALU_DEP_1)
+; GFX11-NEXT:    v_dual_mov_b32 v4, s2 :: v_dual_lshlrev_b32 v3, 3, v2
 ; GFX11-NEXT:    global_load_b64 v[0:1], v3, s[0:1]
+; GFX11-NEXT:    v_mov_b32_dpp v4, v2 quad_perm:[1,0,0,0] row_mask:0xf bank_mask:0xf bound_ctrl:1
 ; GFX11-NEXT:    s_waitcnt vmcnt(0)
-; GFX11-NEXT:    _e64_dpp v[0:1], v2, v[0:1] quad_perm:[1,0,0,0] row_mask:0xf bank_mask:0xf bound_ctrl:1
+; GFX11-NEXT:    v_lshlrev_b64 v[0:1], v4, v[0:1]
 ; GFX11-NEXT:    global_store_b64 v3, v[0:1], s[0:1]
 ; GFX11-NEXT:    s_endpgm
 ;
@@ -1288,12 +1291,13 @@ define amdgpu_kernel void @dpp_shl_i64_amount(ptr addrspace(1) %arg, i32 %in) {
 ; GFX12:       ; %bb.0:
 ; GFX12-NEXT:    s_load_b96 s[0:2], s[4:5], 0x24
 ; GFX12-NEXT:    v_and_b32_e32 v2, 0x3ff, v0
-; GFX12-NEXT:    s_delay_alu instid0(VALU_DEP_1)
-; GFX12-NEXT:    v_lshlrev_b32_e32 v3, 3, v2
 ; GFX12-NEXT:    s_wait_kmcnt 0x0
+; GFX12-NEXT:    s_delay_alu instid0(VALU_DEP_1) | instskip(SKIP_3) | instid1(VALU_DEP_1)
+; GFX12-NEXT:    v_dual_mov_b32 v4, s2 :: v_dual_lshlrev_b32 v3, 3, v2
 ; GFX12-NEXT:    global_load_b64 v[0:1], v3, s[0:1]
+; GFX12-NEXT:    v_mov_b32_dpp v4, v2 quad_perm:[1,0,0,0] row_mask:0xf bank_mask:0xf bound_ctrl:1
 ; GFX12-NEXT:    s_wait_loadcnt 0x0
-; GFX12-NEXT:    v_lshlrev_b64_dpp v[0:1], v2, v[0:1] quad_perm:[1,0,0,0] row_mask:0xf bank_mask:0xf bound_ctrl:1
+; GFX12-NEXT:    v_lshlrev_b64_e32 v[0:1], v4, v[0:1]
 ; GFX12-NEXT:    global_store_b64 v3, v[0:1], s[0:1]
 ; GFX12-NEXT:    s_endpgm
   %id = tail call i32 @llvm.amdgcn.workitem.id.x()

``````````

</details>


https://github.com/llvm/llvm-project/pull/222216