[AMDGPU] "Invalid VOPD pair was created" error with V_FMAMK_F32

[jayfoad]

If I add this test case:

diff --git a/llvm/test/CodeGen/AMDGPU/vopd-combine-gfx1250.mir b/llvm/test/CodeGen/AMDGPU/vopd-combine-gfx1250.mir
index b05edd046b87..93648f9dc0aa 100644
--- a/llvm/test/CodeGen/AMDGPU/vopd-combine-gfx1250.mir
+++ b/llvm/test/CodeGen/AMDGPU/vopd-combine-gfx1250.mir
@@ -4481,3 +4481,12 @@ body: |
       $vgpr3 = V_MOV_B32_e32 0, implicit $exec
       $vgpr0 = V_ADD_F32_e64_dpp $vgpr0, 0, $vgpr2, 0, $vgpr1, 0, 1, 1, 15, 15, 1, implicit $mode, implicit $exec
 ...
+
+---
+name: vopd_no_combine_fmamk_src1
+tracksRegLiveness: true
+body: |
+  bb.0:
+    $vgpr142 = V_FMAMK_F32 $vgpr377, 1069066811, $vgpr142, implicit $mode, implicit $exec
+    $vgpr145 = V_FMAC_F32_e32 1069066811, $vgpr366, $vgpr145, implicit $mode, implicit $exec
+...

it fails with:

Invalid VOPD pair was created
UNREACHABLE executed at /home/jayfoad2/git/llvm-project/llvm/lib/Target/AMDGPU/AMDGPULowerVGPREncoding.cpp:213!

[llvmbot]

@llvm/issue-subscribers-backend-amdgpu

Author: Jay Foad (jayfoad)

If I add this test case: ``` diff --git a/llvm/test/CodeGen/AMDGPU/vopd-combine-gfx1250.mir b/llvm/test/CodeGen/AMDGPU/vopd-combine-gfx1250.mir index b05edd046b87..93648f9dc0aa 100644 --- a/llvm/test/CodeGen/AMDGPU/vopd-combine-gfx1250.mir +++ b/llvm/test/CodeGen/AMDGPU/vopd-combine-gfx1250.mir @@ -4481,3 +4481,12 @@ body: | $vgpr3 = V_MOV_B32_e32 0, implicit $exec $vgpr0 = V_ADD_F32_e64_dpp $vgpr0, 0, $vgpr2, 0, $vgpr1, 0, 1, 1, 15, 15, 1, implicit $mode, implicit $exec ... + +--- +name: vopd_no_combine_fmamk_src1 +tracksRegLiveness: true +body: | + bb.0: + $vgpr142 = V_FMAMK_F32 $vgpr377, 1069066811, $vgpr142, implicit $mode, implicit $exec + $vgpr145 = V_FMAC_F32_e32 1069066811, $vgpr366, $vgpr145, implicit $mode, implicit $exec +... ``` it fails with: ``` Invalid VOPD pair was created UNREACHABLE executed at /home/jayfoad2/git/llvm-project/llvm/lib/Target/AMDGPU/AMDGPULowerVGPREncoding.cpp:213! ```

[jayfoad]

The gcn-create-vopd pass succeeds in combining these two instructions:

  $vgpr142 = V_FMAMK_F32 $vgpr377, 1069066811, $vgpr142, implicit $mode, implicit $exec
  $vgpr145 = V_FMAC_F32_e32 1069066811, $vgpr366, $vgpr145(tied-def 0), implicit $mode, implicit $exec

into this one:

  $vgpr142, $vgpr145 = V_DUAL_FMAMK_F32_X_FMAC_F32_e32_gfx1250 $vgpr377, 1069066811, $vgpr142, 1069066811, $vgpr366, $vgpr145(tied-def 1), implicit $mode, implicit $exec, implicit $mode, implicit $exec, implicit $mode, implicit $exec

[jayfoad]

The problem is that in the VOPD instruction, operands vsrc1X and vsrc1Y have different MSBs. This is detected in AMDGPULowerVGPREncoding::computeMode.

The same check should have been performed before forming the VOPD pair, in InstInfo::getInvalidCompOperandIndex. I think the problem is that this check went wrong because the operands of V_FMAMK_F32 are laid out in a strange order, with src0 and src1 not being consecutive:

  dag InOperandList = (ins VSrc_f32:$src0, KImmFP32:$imm, VGPR_32:$src1);

Maybe ComponentLayout::getIndexOfSrcInMCOperands needs to be taught about this strangeness?

[jayfoad]

@rampitec @Sisyph I am really not familiar with this code and would appreciate any help!

[rampitec]

It is fishy since v_dual_fmamk_f32 is also supported on GFX11. This test hits the MSB check which is only applicable on GFX1250, but if we are using wrong indexes then other checks shall fail for other constraints on GFX11 too.

[rampitec]

I think the pair is formed correctly. According to SPG VOPD constraints:

FMAMK is an exception : V = S0 + K * S1 ("S1" uses the SRC2 read port).

I.e., src1 shall really be treated as src2. Which then means it is MSB checks in the VGPR lowering are not correct.

[rampitec]

The final instruction if we ignore the check is:

v_dual_fmamk_f32 v142, v121 /*v377*/, 0x3fb8aa3b, v142 :: v_dual_fmac_f32 v145, 0x3fb8aa3b, v110 /*v366*/

which looks correct to me.

[rampitec]

In the getVGPRLoweringOperandTables() we are returning:

  if (AMDGPU::isVOPD(Desc.getOpcode()))
    return {VOPDOpsX, VOPDOpsY};

If the a component is FMAMK we shall return a different table.