AMDGPU: Look through bundles for existing waitcnts

[arsenm]

Summary

These aren't produced now, but will be in a future patch.

[kerbowa]

What about the possibility of inserting waitcnt into bundles?

lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1360–1361

Can we just skip BUNDLE instructions.

[arsenm]

Add tests for inter and cross bundle waitcnts. Somewhat surprisingly, works as expected

[arsenm]

lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1360–1361

That's the opposite of the point. The point is to find the waitcnt inside the bundle

[kerbowa]

LGTM but I will wait for Stas.

lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1360–1361

I mean the actual BUNDLE instruction itself, but it's not actually important.

[rampitec]

What will happen if we decide to insert wait in the middle of the bundle? Before this change it was impossible.

[arsenm]

In D64081#1566980, @rampitec wrote:

What will happen if we decide to insert wait in the middle of the bundle? Before this change it was impossible.

That's tested in insert_in_bundle. It seems to work as expected, and puts a waitcnt inside the bundle

[rampitec]

LGTM.

[arsenm]

r364983

lib/Target/AMDGPU/SIInsertWaitcnts.cpp

bool SIInsertWaitcnts::insertWaitcntInBlock(MachineFunction &MF,
   LLVM_DEBUG({
     dbgs() << "*** Block" << Block.getNumber() << " ***";
     ScoreBrackets.dump();
   });
 
   // Walk over the instructions.
   MachineInstr *OldWaitcntInstr = nullptr;
 
-  for (MachineBasicBlock::iterator Iter = Block.begin(), E = Block.end();
+  for (MachineBasicBlock::instr_iterator Iter = Block.instr_begin(),
+                                         E = Block.instr_end();
        Iter != E;) {
     MachineInstr &Inst = *Iter;
 
     // Track pre-existing waitcnts from earlier iterations.
     if (Inst.getOpcode() == AMDGPU::S_WAITCNT ||
         (Inst.getOpcode() == AMDGPU::S_WAITCNT_VSCNT &&
          Inst.getOperand(0).isReg() &&
          Inst.getOperand(0).getReg() == AMDGPU::SGPR_NULL)) {

test/CodeGen/AMDGPU/waitcnt.mir

--- |
   define amdgpu_kernel void @single_fallthrough_successor_no_end_block_wait() {
     ret void
   }
 
   define amdgpu_kernel void @single_branch_successor_not_next_block() {
     ret void
   }
 
+  define amdgpu_kernel void @preexisting_waitcnt() {
+    ret void
+  }
+
+  define amdgpu_kernel void @bundle_no_waitcnt() {
+    ret void
+  }
+
+  define amdgpu_kernel void @preexisting_waitcnt_in_bundle() {
+    ret void
+  }
+
+  define amdgpu_kernel void @insert_in_bundle() {
+    ret void
+  }
+
+  define amdgpu_kernel void @exit_bundle() {
+    ret void
+  }
+
+  define amdgpu_kernel void @cross_bundle() {
+    ret void
+  }
+
 ...
 ---
 
 # CHECK-LABEL: name: flat_zero_waitcnt
 
 # CHECK-LABEL: bb.0:
 # CHECK: FLAT_LOAD_DWORD
 # CHECK: FLAT_LOAD_DWORDX4
bb.1:
     FLAT_STORE_DWORD $vgpr8_vgpr9, $vgpr10, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
     S_ENDPGM 0
 
   bb.2:
      $vgpr3_vgpr4 = V_LSHLREV_B64 4, $vgpr7_vgpr8, implicit $exec
     FLAT_STORE_DWORD $vgpr3_vgpr4, $vgpr0, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
     S_ENDPGM 0
 ...
+
+# CHECK-LABEL: name: preexisting_waitcnt{{$}}
+# CHECK: FLAT_LOAD_DWORD
+# CHECK-NEXT: S_WAITCNT 0
+# CHECK-NOT: S_WAITCNT
+name: preexisting_waitcnt
+tracksRegLiveness: true
+machineFunctionInfo:
+  isEntryFunction: true
+body: |
+  bb.0:
+    liveins: $vgpr1_vgpr2
+    $vgpr0 = FLAT_LOAD_DWORD $vgpr1_vgpr2, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+    S_WAITCNT 0
+    FLAT_STORE_DWORD $vgpr1_vgpr2, $vgpr0, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+
+...
+
+---
+
+# CHECK-LABEL: name: bundle_no_waitcnt{{$}}
+# CHECK: FLAT_LOAD_DWORD
+# CHECK-NEXT: BUNDLE
+# CHECK-NEXT: S_NOP
+# CHECK-NEXT: S_NOP
+# CHECK-NEXT: }
+# CHECK-NEXT: S_WAITCNT 112
+name: bundle_no_waitcnt
+tracksRegLiveness: true
+machineFunctionInfo:
+  isEntryFunction: true
+body: |
+  bb.0:
+    liveins: $vgpr1_vgpr2
+    $vgpr0 = FLAT_LOAD_DWORD $vgpr1_vgpr2, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+    BUNDLE {
+      S_NOP 0
+      S_NOP 0
+    }
+    FLAT_STORE_DWORD $vgpr1_vgpr2, $vgpr0, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+
+...
+
+---
+
+# See the waitcnt inside the bundle and don't insert an extra
+# CHECK-LABEL: name: preexisting_waitcnt_in_bundle{{$}}
+# CHECK: FLAT_LOAD_DWORD
+# CHECK: S_WAITCNT 0
+# CHECK-NOT: S_WAITCNT
+name: preexisting_waitcnt_in_bundle
+tracksRegLiveness: true
+machineFunctionInfo:
+  isEntryFunction: true
+body: |
+  bb.0:
+    liveins: $vgpr1_vgpr2
+    $vgpr0 = FLAT_LOAD_DWORD $vgpr1_vgpr2, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+    BUNDLE {
+      S_NOP 0
+      S_WAITCNT 0
+    }
+    FLAT_STORE_DWORD $vgpr1_vgpr2, $vgpr0, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+
+...
+
+---
+
+# Def and use inside bundle
+# CHECK-LABEL: name: insert_in_bundle{{$}}
+# CHECK: BUNDLE implicit-def $vgpr0, implicit $vgpr1_vgpr2 {
+# CHECK-NEXT: $vgpr0 = FLAT_LOAD_DWORD $vgpr1_vgpr2, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+# CHECK-NEXT: S_WAITCNT 112
+# CHECK-NEXT: FLAT_STORE_DWORD $vgpr1_vgpr2, internal $vgpr0, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+# CHECK-NEXT: }
+
+name: insert_in_bundle
+tracksRegLiveness: true
+machineFunctionInfo:
+  isEntryFunction: true
+body: |
+  bb.0:
+    liveins: $vgpr1_vgpr2
+    BUNDLE implicit-def $vgpr0, implicit $vgpr1_vgpr2 {
+    $vgpr0 = FLAT_LOAD_DWORD $vgpr1_vgpr2, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+    FLAT_STORE_DWORD $vgpr1_vgpr2, internal $vgpr0, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+    }
+...
+
+---
+
+# Def is last instruction in bundle, use is outside bundle
+
+# CHECK-LABEL: name: exit_bundle{{$}}
+# CHECK: BUNDLE implicit-def $vgpr0, implicit $vgpr1_vgpr2 {
+# CHECK-NEXT: $vgpr0 = FLAT_LOAD_DWORD $vgpr1_vgpr2, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+# CHECK-NEXT: }
+# CHECK-NEXT: S_WAITCNT 112
+# CHECK-NEXT: FLAT_STORE_DWORD $vgpr1_vgpr2, $vgpr0, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+
+name: exit_bundle
+tracksRegLiveness: true
+machineFunctionInfo:
+  isEntryFunction: true
+body: |
+  bb.0:
+    liveins: $vgpr1_vgpr2
+    BUNDLE implicit-def $vgpr0, implicit $vgpr1_vgpr2 {
+    $vgpr0 = FLAT_LOAD_DWORD $vgpr1_vgpr2, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+    }
+
+    FLAT_STORE_DWORD $vgpr1_vgpr2, $vgpr0, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+
+...
+
+---
+
+# Def is in bundle, use is in another bundle
+
+# CHECK-LABEL: name: cross_bundle{{$}}
+# CHECK: BUNDLE implicit-def $vgpr0, implicit $vgpr1_vgpr2 {
+# CHECK-NEXT: $vgpr0 = FLAT_LOAD_DWORD $vgpr1_vgpr2, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+# CHECK-NEXT: }
+# CHECK-NEXT: S_WAITCNT 112
+# CHECK-NEXT: BUNDLE implicit $vgpr0, implicit $vgpr1_vgpr2 {
+# CHECK-NEXT: FLAT_STORE_DWORD $vgpr1_vgpr2, $vgpr0, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+# CHECK-NEXT: }
+
+name: cross_bundle
+tracksRegLiveness: true
+machineFunctionInfo:
+  isEntryFunction: true
+body: |
+  bb.0:
+    liveins: $vgpr1_vgpr2
+    BUNDLE implicit-def $vgpr0, implicit $vgpr1_vgpr2 {
+    $vgpr0 = FLAT_LOAD_DWORD $vgpr1_vgpr2, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+    }
+    BUNDLE implicit $vgpr0, implicit $vgpr1_vgpr2 {
+      FLAT_STORE_DWORD $vgpr1_vgpr2, $vgpr0, 0, 0, 0, 0, implicit $exec, implicit $flat_scr
+    }
+...