[AMDGPU] Fix hazard handling of v_cmpx to permlane

[rampitec]

Summary

• VOP3 and SDWA forms of V_CMPX were not handled
• Hazard only exists if the compare defines EXEC (i.e. V_CMPX) forwarded to the permlane.

[rampitec]

Split of sdwa test, it is not supported on gfx1100.

@Joe_Nash, @foad actually is this hazard even exists on gfx11?

[rampitec]

In D127344#3568055, @rampitec wrote:

Split of sdwa test, it is not supported on gfx1100.

@Joe_Nash, @foad actually is this hazard even exists on gfx11?

As far as I can tell it does not exist already in the gfx1030. Is there any reason gfx1100 has FeatureVcmpxPermlaneHazard?

[rampitec]

In D127344#3568087, @rampitec wrote:

In D127344#3568055, @rampitec wrote:

Split of sdwa test, it is not supported on gfx1100.

@Joe_Nash, @foad actually is this hazard even exists on gfx11?

As far as I can tell it does not exist already in the gfx1030. Is there any reason gfx1100 has FeatureVcmpxPermlaneHazard?

OK, found it in the gfx11 spec.

[foad]

LGTM as-is, but perhaps it is time to introduce an "IsCMPX" helper function?

Does this also handle DPP forms of compares?

Maybe add a negative test for v_cmp->permlane since that is part of what you have fixed.

[foad]

Does this also handle DPP forms of compares?

Oh I forgot, these don't exist yet, until D126989 is landed.

[rampitec]

In D127344#3569337, @foad wrote:

LGTM as-is, but perhaps it is time to introduce an "IsCMPX" helper function?

Does this also handle DPP forms of compares?

Maybe add a negative test for v_cmp->permlane since that is part of what you have fixed.

There is no DPP form because gfx11 VOPC DPP is not yet sumbitted AFAIR.

I didn't want to add a negative test because even initial form was very conservative. It is almost impossible to run into that hazard, so the check was very relaxed and catching more than needed. I'd prefer to add a negative test after gfx11 code for this is settled, as we yet didn't agree on the tsflag bits for that.

[rampitec]

As a side note: I tend to distrust documentation. This is documented as affecting 1000-1014 as a bug, then disappear, then appear again as an official hazard in 1100. So I have a good reason to think it also exists everywhere between 1014 and 1100 as well. But this is only my speculation.

[Joe_Nash]

LGTM

llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp

void GCNHazardRecognizer::fixHazards(MachineInstr *MI) {
   fixLdsBranchVmemWARHazard(MI);
 }
 
 bool GCNHazardRecognizer::fixVcmpxPermlaneHazards(MachineInstr *MI) {
   if (!ST.hasVcmpxPermlaneHazard() || !isPermlane(*MI))
     return false;
 
   const SIInstrInfo *TII = ST.getInstrInfo();
-  auto IsHazardFn = [TII](const MachineInstr &MI) { return TII->isVOPC(MI); };
+  const SIRegisterInfo *TRI = ST.getRegisterInfo();
+  auto IsHazardFn = [TII, TRI](const MachineInstr &MI) {
+    return (TII->isVOPC(MI) ||
+            ((TII->isVOP3(MI) || TII->isSDWA(MI)) && MI.isCompare())) &&
+           MI.modifiesRegister(AMDGPU::EXEC, TRI);
+  };
 
   auto IsExpiredFn = [](const MachineInstr &MI, int) {
     unsigned Opc = MI.getOpcode();
     return SIInstrInfo::isVALU(MI) && Opc != AMDGPU::V_NOP_e32 &&
            Opc != AMDGPU::V_NOP_e64 && Opc != AMDGPU::V_NOP_sdwa;
   };
 
   if (::getWaitStatesSince(IsHazardFn, MI, IsExpiredFn) ==

llvm/test/CodeGen/AMDGPU/vcmpx-permlane-hazard-sdwa.mir

+# RUN: llc -march=amdgcn -mcpu=gfx1010 -verify-machineinstrs -run-pass post-RA-hazard-rec -o - %s | FileCheck -check-prefix=GCN %s
+
+# GCN-LABEL: name: hazard_vcmpx_sdwa_permlane16
+# GCN:      V_CMPX_LE_F32_nosdst_sdwa
+# GCN:      S_ADD_U32
+# GCN-NEXT: $vgpr1 = V_MOV_B32_e32 killed $vgpr1, implicit $exec
+# GCN-NEXT: V_PERMLANE16_B32_e64
+---
+name:            hazard_vcmpx_sdwa_permlane16
+body:            |
+  bb.0:
+    successors: %bb.1
+    $vgpr0 = V_MOV_B32_e32 0, implicit $exec
+    V_CMPX_LE_F32_nosdst_sdwa 0, $vgpr0, 0, $vgpr0, 0, 0, implicit-def $exec, implicit $mode, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    $vgpr1 = IMPLICIT_DEF
+    $vgpr2 = IMPLICIT_DEF
+    $sgpr0 = IMPLICIT_DEF
+    $sgpr1 = S_ADD_U32 $sgpr0, 0, implicit-def $scc
+    $vgpr1 = V_PERMLANE16_B32_e64 0, killed $vgpr1, 0, killed $sgpr1, 0, killed $sgpr0, $vgpr1, 0, implicit $exec
+    S_ENDPGM 0
+...

llvm/test/CodeGen/AMDGPU/vcmpx-permlane-hazard.mir

body:            |
 
   bb.1:
     $vgpr2 = IMPLICIT_DEF
     $sgpr0 = IMPLICIT_DEF
     $sgpr1 = S_ADD_U32 $sgpr0, 0, implicit-def $scc
     $vgpr1 = V_PERMLANE16_B32_e64 0, undef $vgpr1, 0, killed $sgpr1, 0, killed $sgpr0, undef $vgpr1, 0, implicit $exec
     S_ENDPGM 0
 ...
+
+# GCN-LABEL: name: hazard_vcmpx_e64_permlane16
+# GCN:      V_CMPX_LE_F32_nosdst_e64
+# GCN:      S_ADD_U32
+# GCN-NEXT: $vgpr1 = V_MOV_B32_e32 killed $vgpr1, implicit $exec
+# GCN-NEXT: V_PERMLANE16_B32_e64
+---
+name:            hazard_vcmpx_e64_permlane16
+body:            |
+  bb.0:
+    successors: %bb.1
+    $vgpr0 = V_MOV_B32_e32 0, implicit $exec
+    V_CMPX_LE_F32_nosdst_e64 0, 0, 0, $vgpr0, 0, implicit-def $exec, implicit $mode, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    $vgpr1 = IMPLICIT_DEF
+    $vgpr2 = IMPLICIT_DEF
+    $sgpr0 = IMPLICIT_DEF
+    $sgpr1 = S_ADD_U32 $sgpr0, 0, implicit-def $scc
+    $vgpr1 = V_PERMLANE16_B32_e64 0, killed $vgpr1, 0, killed $sgpr1, 0, killed $sgpr0, $vgpr1, 0, implicit $exec
+    S_ENDPGM 0
+...