[AMDGPU] Fix inserting combined s_nop in bundles

[rampitec]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.ds.gws.sema.p.ll

14

Is this expected?

[rampitec]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.ds.gws.sema.p.ll

14

According to GCNHazardRecognizer::checkRFEHazards() it needs 1 waitstate.

[kerbowa]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.ds.gws.sema.p.ll

14

I don't think this is an RFE hazard.

[rampitec]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.ds.gws.sema.p.ll

14

Hm... Right, there is no rfe here...

[rampitec]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.ds.gws.sema.p.ll

14

OK, it was inserted by checkReadM0Hazards(). There is a def and use of m0, but it seem to incorrectly process BOUNDLE itself. There shall be 1 waitstate between def and use of m0. PreEmitNoopsCommon() returns 1 from checkReadM0Hazards(), and the IR is:

bb.0 (%ir-block.0):
  successors: %bb.1(0x80000000); %bb.1(100.00%)

  $m0 = S_MOV_B32 0

bb.1:
; predecessors: %bb.1, %bb.0
  successors: %bb.1(0x40000000), %bb.2(0x40000000); %bb.1(50.00%), %bb.2(50.00%)

  S_SETREG_IMM32_B32 0, 515, implicit-def $mode, implicit $mode
  BUNDLE implicit $m0, implicit $exec {
    DS_GWS_SEMA_V 0, -1, implicit $m0, implicit $exec :: (store 4 into custom "GWSResource")
    S_WAITCNT 0
  }
  renamable $sgpr0 = S_GETREG_B32 515
  S_CMP_LG_U32 killed renamable $sgpr0, 0, implicit-def $scc
  S_CBRANCH_SCC1 %bb.1, implicit killed $scc

There is def of m0 in bb.0, but it is really expired by the GWS operation.

[rampitec]

LGTM

[kerbowa]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.ds.gws.sema.p.ll

14

Yes, the redundant nop is a consequence of the order that the regions scheduled. MMB are visited in order but regions are discovered bottom-up within a block. The S_SETREG is not seen by the hazard recognizer until after the bundle.

It seems like we may want to reset the tracked instructions for hazards when starting scheduling on a new region.

llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp

if ((SIInstrInfo::isVMEM(*MI) ||
     return HazardType;
 
   if (MI->isInlineAsm() && checkInlineAsmHazards(MI) > 0)
     return HazardType;
 
   return NoHazard;
 }
 
-static void insertNoopInBundle(MachineInstr *MI, const SIInstrInfo &TII) {
+static void insertNoopsInBundle(MachineInstr *MI, const SIInstrInfo &TII,
+                                unsigned Quantity) {
+  while (Quantity > 0) {
+    unsigned Arg;
+    if (Quantity >= 8)
+      Arg = 7;
+    else
+      Arg = Quantity - 1;
+    Quantity -= Arg + 1;
     BuildMI(*MI->getParent(), MI, MI->getDebugLoc(), TII.get(AMDGPU::S_NOP))
-      .addImm(0);
+        .addImm(Arg);
+  }
 }
 
 void GCNHazardRecognizer::processBundle() {
   MachineBasicBlock::instr_iterator MI = std::next(CurrCycleInstr->getIterator());
   MachineBasicBlock::instr_iterator E = CurrCycleInstr->getParent()->instr_end();
   // Check bundled MachineInstr's for hazards.
   for (; MI != E && MI->isInsideBundle(); ++MI) {
     CurrCycleInstr = &*MI;
     unsigned WaitStates = PreEmitNoopsCommon(CurrCycleInstr);
 
-    if (IsHazardRecognizerMode)
+    if (IsHazardRecognizerMode) {
       fixHazards(CurrCycleInstr);
 
-    for (unsigned i = 0; i < WaitStates; ++i)
-      insertNoopInBundle(CurrCycleInstr, TII);
+      insertNoopsInBundle(CurrCycleInstr, TII, WaitStates);
+    }
 
     // It’s unnecessary to track more than MaxLookAhead instructions. Since we
     // include the bundled MI directly after, only add a maximum of
     // (MaxLookAhead - 1) noops to EmittedInstrs.
     for (unsigned i = 0, e = std::min(WaitStates, MaxLookAhead - 1); i < e; ++i)
       EmittedInstrs.push_front(nullptr);
 
     EmittedInstrs.push_front(CurrCycleInstr);

llvm/test/CodeGen/AMDGPU/hazard-in-bundle.mir

BUNDLE {
       DBG_LABEL 3
       DBG_LABEL 4
       DBG_LABEL 5
       DBG_LABEL 6
       S_SENDMSG 3, implicit $exec, implicit $m0
     }
     S_ENDPGM 0
 ...
+
+# GCN-LABEL: name: vmem_vcc_hazard_in_bundle
+# GCN:      S_LOAD_DWORDX2_IMM
+# GCN-NEXT: S_NOP 3
+# GCN:      BUFFER_LOAD_DWORD_OFFEN
+---
+name: vmem_vcc_hazard_in_bundle
+body: |
+  bb.0:
+    BUNDLE {
+      $sgpr0_sgpr1_sgpr2_sgpr3 = IMPLICIT_DEF
+      $vgpr0 = IMPLICIT_DEF
+      $vgpr1 = V_ADDC_U32_e32 $vgpr0, $vgpr0, implicit-def $vcc, implicit $vcc, implicit $exec
+      $sgpr0_sgpr1 = S_LOAD_DWORDX2_IMM $sgpr10_sgpr11, 0, 0, 0
+      $vgpr1 = BUFFER_LOAD_DWORD_OFFEN $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, $vcc_lo, 0, 0, 0, 0, 0, 0, implicit $exec
+    }
+    S_ENDPGM 0
+...

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.ds.gws.sema.p.ll

 ; RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=tahiti -o - -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,LOOP %s
 ; RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=hawaii -o - -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,LOOP %s
 ; RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=fiji -o - -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,LOOP,GFX8 %s
 ; RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=gfx900 -o - -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,NOLOOP %s
 ; RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=gfx1010 -o - -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,NOLOOP,GFX10 %s
 
 ; GCN-LABEL: {{^}}gws_sema_p_offset0:
 ; NOLOOP-DAG: s_mov_b32 m0, 0{{$}}
 ; NOLOOP: ds_gws_sema_p gds{{$}}
 
 ; LOOP: s_mov_b32 m0, 0{{$}}
 ; LOOP: [[LOOP:BB[0-9]+_[0-9]+]]:
 ; LOOP-NEXT: s_setreg_imm32_b32 hwreg(HW_REG_TRAPSTS, 8, 1), 0
-; GFX8-NEXT: s_nop 0
 ; LOOP-NEXT: ds_gws_sema_p gds
 ; LOOP-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; LOOP-NEXT: s_getreg_b32 [[GETREG:s[0-9]+]], hwreg(HW_REG_TRAPSTS, 8, 1)
 ; LOOP-NEXT: s_cmp_lg_u32 [[GETREG]], 0
 ; LOOP-NEXT: s_cbranch_scc1 [[LOOP]]
 define amdgpu_kernel void @gws_sema_p_offset0(i32 %val) #0 {
   call void @llvm.amdgcn.ds.gws.sema.p(i32 0)
   ret void
 }
 
 declare void @llvm.amdgcn.ds.gws.sema.p(i32) #0
 
 attributes #0 = { convergent inaccessiblememonly nounwind }

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.ds.gws.sema.release.all.ll

 
 ; GCN-LABEL: {{^}}gws_sema_release_all_offset0:
 ; NOLOOP-DAG: s_mov_b32 m0, 0{{$}}
 ; NOLOOP: ds_gws_sema_release_all gds{{$}}
 
 ; LOOP: s_mov_b32 m0, 0{{$}}
 ; LOOP: [[LOOP:BB[0-9]+_[0-9]+]]:
 ; LOOP-NEXT: s_setreg_imm32_b32 hwreg(HW_REG_TRAPSTS, 8, 1), 0
-; GFX8-NEXT: s_nop 0
 ; LOOP-NEXT: ds_gws_sema_release_all gds
 ; LOOP-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; LOOP-NEXT: s_getreg_b32 [[GETREG:s[0-9]+]], hwreg(HW_REG_TRAPSTS, 8, 1)
 ; LOOP-NEXT: s_cmp_lg_u32 [[GETREG]], 0
 ; LOOP-NEXT: s_cbranch_scc1 [[LOOP]]
 define amdgpu_kernel void @gws_sema_release_all_offset0(i32 %val) #0 {
   call void @llvm.amdgcn.ds.gws.sema.release.all(i32 0)
   ret void
 }
 
 declare void @llvm.amdgcn.ds.gws.sema.release.all(i32) #0
 
 attributes #0 = { convergent inaccessiblememonly nounwind }

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.ds.gws.sema.v.ll

 ; RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=tahiti -o - -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,LOOP %s
 ; RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=hawaii -o - -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,LOOP %s
 ; RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=fiji -o - -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,LOOP,GFX8 %s
 ; RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=gfx900 -o - -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,NOLOOP %s
 ; RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=gfx1010 -o - -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,NOLOOP,GFX10 %s
 
 ; GCN-LABEL: {{^}}gws_sema_v_offset0:
 ; NOLOOP-DAG: s_mov_b32 m0, 0{{$}}
 ; NOLOOP: ds_gws_sema_v gds{{$}}
 
 ; LOOP: s_mov_b32 m0, 0{{$}}
 ; LOOP: [[LOOP:BB[0-9]+_[0-9]+]]:
 ; LOOP-NEXT: s_setreg_imm32_b32 hwreg(HW_REG_TRAPSTS, 8, 1), 0
-; GFX8-NEXT: s_nop 0
 ; LOOP-NEXT: ds_gws_sema_v gds
 ; LOOP-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; LOOP-NEXT: s_getreg_b32 [[GETREG:s[0-9]+]], hwreg(HW_REG_TRAPSTS, 8, 1)
 ; LOOP-NEXT: s_cmp_lg_u32 [[GETREG]], 0
 ; LOOP-NEXT: s_cbranch_scc1 [[LOOP]]
 define amdgpu_kernel void @gws_sema_v_offset0(i32 %val) #0 {
   call void @llvm.amdgcn.ds.gws.sema.v(i32 0)
   ret void
 }
 
 declare void @llvm.amdgcn.ds.gws.sema.v(i32) #0
 
 attributes #0 = { convergent inaccessiblememonly nounwind }