[AMDGPU] Move insertion of function entry waitcnt later

[kerbowa]

Summary

This allows tracking these as preexisting waitcnt.

[arsenm]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1605

I thought we made this explicitly a property of the calling convention, so we could have this not be inserted for future conventions. I guess if this isn't checking the function for that already, it's a preexisting problem

llvm/test/CodeGen/AMDGPU/waitcnt-preexisting.mir

188–190

-NEXTs would be a bit more dependable

[rampitec]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1603

Modified = false;?

1610

Is this comment still relevant?

[foad]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1607

Typo, should be "to wait" or "to do the wait"

[kerbowa]

Address comments.

[kerbowa]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1605

When would that waitcnt be added if it's a property of the CC? It seems like these changes could help avoid it being added twice since they would be combined.

1610

It might be, if what it's saying is that we could move this waitcnt later until the function uses callee saved registers.

[rampitec]

LGTM

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

bool SIInsertWaitcnts::runOnMachineFunction(MachineFunction &MF) {
 
   RegisterEncoding.VGPR0 = TRI->getEncodingValue(AMDGPU::VGPR0);
   RegisterEncoding.VGPRL = RegisterEncoding.VGPR0 + NumVGPRsMax - 1;
   RegisterEncoding.SGPR0 = TRI->getEncodingValue(AMDGPU::SGPR0);
   RegisterEncoding.SGPRL = RegisterEncoding.SGPR0 + NumSGPRsMax - 1;
 
   TrackedWaitcntSet.clear();
   BlockInfos.clear();
+  bool Modified = false;
+
+  if (!MFI->isEntryFunction()) {
+    // Wait for any outstanding memory operations that the input registers may
+    // depend on. We can't track them and it's better to do the wait after the
+    // costly call sequence.
+
+    // TODO: Could insert earlier and schedule more liberally with operations
+    // that only use caller preserved registers.
+    MachineBasicBlock &EntryBB = MF.front();
+    MachineBasicBlock::iterator I = EntryBB.begin();
+    for (MachineBasicBlock::iterator E = EntryBB.end();
+         I != E && (I->isPHI() || I->isMetaInstruction()); ++I)
+      ;
+    BuildMI(EntryBB, I, DebugLoc(), TII->get(AMDGPU::S_WAITCNT)).addImm(0);
+    if (ST->hasVscnt())
+      BuildMI(EntryBB, I, DebugLoc(), TII->get(AMDGPU::S_WAITCNT_VSCNT))
+          .addReg(AMDGPU::SGPR_NULL, RegState::Undef)
+          .addImm(0);
+
+    Modified = true;
+  }
 
   // Keep iterating over the blocks in reverse post order, inserting and
   // updating s_waitcnt where needed, until a fix point is reached.
   for (auto *MBB : ReversePostOrderTraversal<MachineFunction *>(&MF))
     BlockInfos.insert({MBB, BlockInfo(MBB)});
 
   std::unique_ptr<WaitcntBrackets> Brackets;
-  bool Modified = false;
   bool Repeat;
   do {
     Repeat = false;
 
     for (auto BII = BlockInfos.begin(), BIE = BlockInfos.end(); BII != BIE;
          ++BII) {
       BlockInfo &BI = BII->second;
       if (!BI.Dirty)
for (MachineBasicBlock *MBB : EndPgmBlocks) {
             !SeenDCacheWB) {
           Modified = true;
           BuildMI(*MBB, I, I->getDebugLoc(), TII->get(AMDGPU::S_DCACHE_WB));
         }
       }
     }
   }
 
-  if (!MFI->isEntryFunction()) {
-    // Wait for any outstanding memory operations that the input registers may
-    // depend on. We can't track them and it's better to the wait after the
-    // costly call sequence.
-
-    // TODO: Could insert earlier and schedule more liberally with operations
-    // that only use caller preserved registers.
-    MachineBasicBlock &EntryBB = MF.front();
-    MachineBasicBlock::iterator I = EntryBB.begin();
-    for (MachineBasicBlock::iterator E = EntryBB.end();
-         I != E && (I->isPHI() || I->isMetaInstruction()); ++I)
-      ;
-    BuildMI(EntryBB, I, DebugLoc(), TII->get(AMDGPU::S_WAITCNT)).addImm(0);
-    if (ST->hasVscnt())
-      BuildMI(EntryBB, I, DebugLoc(), TII->get(AMDGPU::S_WAITCNT_VSCNT))
-          .addReg(AMDGPU::SGPR_NULL, RegState::Undef)
-          .addImm(0);
-
-    Modified = true;
-  }
-
   return Modified;
 }

llvm/test/CodeGen/AMDGPU/waitcnt-preexisting.mir

 
 ---
 name:            test_waitcnt_preexisting_ignore_kill
 body:             |
   bb.0:
     liveins: $vgpr0_vgpr1
 
     ; GFX9-LABEL: name: test_waitcnt_preexisting_ignore_kill
     ; GFX9: S_WAITCNT 0
     ; GFX9: $vgpr0 = FLAT_LOAD_DWORD $vgpr0_vgpr1, 0, 0, implicit $exec, implicit $flat_scr
     ; GFX9: S_WAITCNT 3952
     ; GFX9: KILL $vgpr0
     $vgpr0 = FLAT_LOAD_DWORD $vgpr0_vgpr1, 0, 0, implicit $exec, implicit $flat_scr
     S_WAITCNT 3952
     KILL $vgpr0
+
+# Combine preexisting waitcnt with wait added to the start of a non-entry function.
+
+---
+name:            test_waitcnt_preexisting_func_start
+body:             |
+  bb.0:
+    ; GFX9-LABEL: name: test_waitcnt_preexisting_func_start
+    ; GFX9: S_WAITCNT 0
+    ; GFX9-NOT: S_WAITCNT 0
+    ; GFX9: S_ENDPGM 0
+    S_WAITCNT 0
+    S_ENDPGM 0
 ...