[AMDGPU] Add missing hazard for DPP-after-EXEC-write

[cwabbott]

Summary

Following the docs, we need at least 5 wait states between an EXEC write
and an instruction that uses DPP.

[arsenm]

Needs test

[cwabbott]

Add a test for the problem as well as a missing test for DPP-after-VGPR-write.

[cwabbott]

Ping.

[arsenm]

test/CodeGen/AMDGPU/inserted-wait-states.mir

495 ↗

(On Diff #104793)

Should check the NOP argument is 0. Ideally these would be combined into one nop with the right value later. Also why not use VI-NEXT for all of them?

[cwabbott]

test/CodeGen/AMDGPU/inserted-wait-states.mir

495 ↗

(On Diff #104793)

Sure, I can check that it's 0. Btw, the rest of this file is inconsistent about whether it checks for a 0 argument or not. I didn't use VI-NEXT to be consistent with the rest of the file which doesn't either, but I can change that too.

And yeah, all these NOP's are combined into one by a later pass; I don't remember exactly which one.

[cwabbott]

• Check for correct S_NOP argument and use VI-NEXT
• Only check for VALU EXEC write as per the docs, and update test

[cwabbott]

Ping.

llvm/trunk/lib/Target/AMDGPU/GCNHazardRecognizer.cpp

int WaitStatesNeededForUse =
         VmemSgprWaitStates - getWaitStatesSinceDef(Use.getReg(), IsHazardDefFn);
     WaitStatesNeeded = std::max(WaitStatesNeeded, WaitStatesNeededForUse);
   }
   return WaitStatesNeeded;
 }
 
 int GCNHazardRecognizer::checkDPPHazards(MachineInstr *DPP) {
   const SIRegisterInfo *TRI = ST.getRegisterInfo();
+  const SIInstrInfo *TII = ST.getInstrInfo();
 
-  // Check for DPP VGPR read after VALU VGPR write.
+  // Check for DPP VGPR read after VALU VGPR write and EXEC write.
   int DppVgprWaitStates = 2;
+  int DppExecWaitStates = 5;
   int WaitStatesNeeded = 0;
+  auto IsHazardDefFn = [TII] (MachineInstr *MI) { return TII->isVALU(*MI); };
 
   for (const MachineOperand &Use : DPP->uses()) {
     if (!Use.isReg() || !TRI->isVGPR(MF.getRegInfo(), Use.getReg()))
       continue;
     int WaitStatesNeededForUse =
         DppVgprWaitStates - getWaitStatesSinceDef(Use.getReg());
     WaitStatesNeeded = std::max(WaitStatesNeeded, WaitStatesNeededForUse);
   }
 
+  WaitStatesNeeded = std::max(
+      WaitStatesNeeded,
+      DppExecWaitStates - getWaitStatesSinceDef(AMDGPU::EXEC, IsHazardDefFn));
+
   return WaitStatesNeeded;
 }
 
 int GCNHazardRecognizer::checkDivFMasHazards(MachineInstr *DivFMas) {
   const SIInstrInfo *TII = ST.getInstrInfo();
 
   // v_div_fmas requires 4 wait states after a write to vcc from a VALU
   // instruction.

llvm/trunk/test/CodeGen/AMDGPU/inserted-wait-states.mir

 # RUN: llc -march=amdgcn -mcpu=tahiti -run-pass post-RA-hazard-rec  %s -o - | FileCheck %s -check-prefixes=GCN
 # RUN: llc -march=amdgcn -mcpu=hawaii -run-pass post-RA-hazard-rec  %s -o - | FileCheck %s -check-prefixes=GCN,CIVI
 # RUN: llc -march=amdgcn -mcpu=fiji -run-pass post-RA-hazard-rec  %s -o - | FileCheck %s -check-prefixes=GCN,CIVI,VI
 # RUN: llc -march=amdgcn -mcpu=gfx900 -run-pass post-RA-hazard-rec  %s -o - | FileCheck %s -check-prefixes=GCN,CIVI,VI,GFX9
 
 --- |
   define amdgpu_kernel void @div_fmas() { ret void }
   define amdgpu_kernel void @s_getreg() { ret void }
   define amdgpu_kernel void @s_setreg() { ret void }
   define amdgpu_kernel void @vmem_gt_8dw_store() { ret void }
   define amdgpu_kernel void @readwrite_lane() { ret void }
   define amdgpu_kernel void @rfe() { ret void }
   define amdgpu_kernel void @s_mov_fed_b32() { ret void }
   define amdgpu_kernel void @s_movrel() { ret void }
   define amdgpu_kernel void @v_interp() { ret void }
+  define amdgpu_kernel void @dpp() { ret void }
 
   define amdgpu_kernel void @mov_fed_hazard_crash_on_dbg_value(i32 addrspace(1)* %A) {
   entry:
     %A.addr = alloca i32 addrspace(1)*, align 4
     store i32 addrspace(1)* %A, i32 addrspace(1)** %A.addr, align 4
     call void @llvm.dbg.declare(metadata i32 addrspace(1)** %A.addr, metadata !5, metadata !11), !dbg !12
     ret void
   }
bb.2:
     %vgpr0 = V_INTERP_P1_F32_16bank %vgpr0, 0, 0, implicit %m0, implicit %exec
     S_BRANCH %bb.3
 
   bb.3:
     %m0 = S_MOV_B32 0
     %vgpr0 = V_INTERP_MOV_F32 0, 0, 0, implicit %m0, implicit %exec
     S_ENDPGM
 ...
+
+...
+---
+
+# GCN-LABEL: name: dpp
+
+# VI-LABEL: bb.0:
+# VI: V_MOV_B32_e32
+# VI-NEXT: S_NOP 0
+# VI-NEXT: S_NOP 0
+# VI-NEXT: V_MOV_B32_dpp
+
+# VI-LABEL: bb.1:
+# VI: V_CMPX_EQ_I32_e32
+# VI-NEXT: S_NOP 0
+# VI-NEXT: S_NOP 0
+# VI-NEXT: S_NOP 0
+# VI-NEXT: S_NOP 0
+# VI-NEXT: S_NOP 0
+# VI-NEXT: V_MOV_B32_dpp
+
+name: dpp
+
+body: |
+  bb.0:
+    %vgpr0 = V_MOV_B32_e32 0, implicit %exec
+    %vgpr1 = V_MOV_B32_dpp %vgpr0, 0, 15, 15, 0, implicit %exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    implicit %exec, implicit %vcc = V_CMPX_EQ_I32_e32 %vgpr0, %vgpr1, implicit %exec
+    %vgpr3 = V_MOV_B32_dpp %vgpr0, 0, 15, 15, 0, implicit %exec
+    S_ENDPGM
+...
 ---
 name:            mov_fed_hazard_crash_on_dbg_value
 alignment:       0
 exposesReturnsTwice: false
 legalized:       false
 regBankSelected: false
 selected:        false
 tracksRegLiveness: true