[llvm] [AMDGPU][SIInsertWaitcnts][NFC] Move instr events code into separate function (PR #180864)

[via llvm-commits]

================
@@ -2660,120 +2662,113 @@ bool SIInsertWaitcnts::insertForcedWaitAfter(MachineInstr &Inst,
   return Result;
 }
 
-void SIInsertWaitcnts::updateEventWaitcntAfter(MachineInstr &Inst,
-                                               WaitcntBrackets *ScoreBrackets) {
-  // Now look at the instruction opcode. If it is a memory access
-  // instruction, update the upper-bound of the appropriate counter's
-  // bracket and the destination operand scores.
-  // For architectures with X_CNT, mark the source address operands
-  // with the appropriate counter values.
-  // TODO: Use the (TSFlags & SIInstrFlags::DS_CNT) property everywhere.
-
-  bool IsVMEMAccess = false;
-  bool IsSMEMAccess = false;
-
+WaitEventSet SIInsertWaitcnts::getEventsFor(const MachineInstr &Inst) const {
+  WaitEventSet Events;
   if (IsExpertMode) {
     if (const auto ET = getExpertSchedulingEventType(Inst))
-      ScoreBrackets->updateByEvent(*ET, Inst);
+      Events.insert(*ET);
   }
 
   if (TII->isDS(Inst) && TII->usesLGKM_CNT(Inst)) {
     if (TII->isAlwaysGDS(Inst.getOpcode()) ||
         TII->hasModifiersSet(Inst, AMDGPU::OpName::gds)) {
-      ScoreBrackets->updateByEvent(GDS_ACCESS, Inst);
-      ScoreBrackets->updateByEvent(GDS_GPR_LOCK, Inst);
-      ScoreBrackets->setPendingGDS();
+      Events.insert(GDS_ACCESS);
+      Events.insert(GDS_GPR_LOCK);
     } else {
-      ScoreBrackets->updateByEvent(LDS_ACCESS, Inst);
+      Events.insert(LDS_ACCESS);
     }
   } else if (TII->isFLAT(Inst)) {
     if (SIInstrInfo::isGFX12CacheInvOrWBInst(Inst.getOpcode())) {
-      ScoreBrackets->updateByEvent(getVmemWaitEventType(Inst), Inst);
-      return;
-    }
-
-    assert(Inst.mayLoadOrStore());
-
-    int FlatASCount = 0;
-
-    if (TII->mayAccessVMEMThroughFlat(Inst)) {
-      ++FlatASCount;
-      IsVMEMAccess = true;
-      ScoreBrackets->updateByEvent(getVmemWaitEventType(Inst), Inst);
-    }
-
-    if (TII->mayAccessLDSThroughFlat(Inst)) {
-      ++FlatASCount;
-      ScoreBrackets->updateByEvent(LDS_ACCESS, Inst);
+      Events.insert(getVmemWaitEventType(Inst));
+    } else {
+      assert(Inst.mayLoadOrStore());
+      if (TII->mayAccessVMEMThroughFlat(Inst)) {
+        if (ST->hasWaitXcnt())
+          Events.insert(VMEM_GROUP);
+        Events.insert(getVmemWaitEventType(Inst));
+      }
+      if (TII->mayAccessLDSThroughFlat(Inst))
+        Events.insert(LDS_ACCESS);
     }
-
-    // Async/LDSDMA operations have FLAT encoding but do not actually use flat
-    // pointers. They do have two operands that each access global and LDS, thus
-    // making it appear at this point that they are using a flat pointer. Filter
-    // them out, and for the rest, generate a dependency on flat pointers so
-    // that both VM and LGKM counters are flushed.
-    if (!SIInstrInfo::isLDSDMA(Inst) && FlatASCount > 1)
-      ScoreBrackets->setPendingFlat();
   } else if (SIInstrInfo::isVMEM(Inst) &&
              (!AMDGPU::getMUBUFIsBufferInv(Inst.getOpcode()) ||
               Inst.getOpcode() == AMDGPU::BUFFER_WBL2)) {
     // BUFFER_WBL2 is included here because unlike invalidates, has to be
     // followed "S_WAITCNT vmcnt(0)" is needed after to ensure the writeback has
     // completed.
-    IsVMEMAccess = true;
-    ScoreBrackets->updateByEvent(getVmemWaitEventType(Inst), Inst);
-
+    if (ST->hasWaitXcnt())
+      Events.insert(VMEM_GROUP);
+    Events.insert(getVmemWaitEventType(Inst));
     if (ST->vmemWriteNeedsExpWaitcnt() &&
         (Inst.mayStore() || SIInstrInfo::isAtomicRet(Inst))) {
-      ScoreBrackets->updateByEvent(VMW_GPR_LOCK, Inst);
+      Events.insert(VMW_GPR_LOCK);
     }
   } else if (TII->isSMRD(Inst)) {
-    IsSMEMAccess = true;
-    ScoreBrackets->updateByEvent(SMEM_ACCESS, Inst);
-  } else if (Inst.isCall()) {
-    // Act as a wait on everything
-    ScoreBrackets->applyWaitcnt(WCG->getAllZeroWaitcnt(/*IncludeVSCnt=*/false));
-    ScoreBrackets->setStateOnFunctionEntryOrReturn();
+    if (ST->hasWaitXcnt())
+      Events.insert(SMEM_GROUP);
+    Events.insert(SMEM_ACCESS);
   } else if (SIInstrInfo::isLDSDIR(Inst)) {
-    ScoreBrackets->updateByEvent(EXP_LDS_ACCESS, Inst);
-  } else if (TII->isVINTERP(Inst)) {
-    int64_t Imm = TII->getNamedOperand(Inst, AMDGPU::OpName::waitexp)->getImm();
-    ScoreBrackets->applyWaitcnt(EXP_CNT, Imm);
+    Events.insert(EXP_LDS_ACCESS);
   } else if (SIInstrInfo::isEXP(Inst)) {
     unsigned Imm = TII->getNamedOperand(Inst, AMDGPU::OpName::tgt)->getImm();
     if (Imm >= AMDGPU::Exp::ET_PARAM0 && Imm <= AMDGPU::Exp::ET_PARAM31)
-      ScoreBrackets->updateByEvent(EXP_PARAM_ACCESS, Inst);
+      Events.insert(EXP_PARAM_ACCESS);
     else if (Imm >= AMDGPU::Exp::ET_POS0 && Imm <= AMDGPU::Exp::ET_POS_LAST)
-      ScoreBrackets->updateByEvent(EXP_POS_ACCESS, Inst);
+      Events.insert(EXP_POS_ACCESS);
     else
-      ScoreBrackets->updateByEvent(EXP_GPR_LOCK, Inst);
+      Events.insert(EXP_GPR_LOCK);
   } else if (SIInstrInfo::isSBarrierSCCWrite(Inst.getOpcode())) {
-    ScoreBrackets->updateByEvent(SCC_WRITE, Inst);
+    Events.insert(SCC_WRITE);
   } else {
     switch (Inst.getOpcode()) {
     case AMDGPU::S_SENDMSG:
     case AMDGPU::S_SENDMSG_RTN_B32:
     case AMDGPU::S_SENDMSG_RTN_B64:
     case AMDGPU::S_SENDMSGHALT:
-      ScoreBrackets->updateByEvent(SQ_MESSAGE, Inst);
+      Events.insert(SQ_MESSAGE);
       break;
     case AMDGPU::S_MEMTIME:
     case AMDGPU::S_MEMREALTIME:
     case AMDGPU::S_GET_BARRIER_STATE_M0:
     case AMDGPU::S_GET_BARRIER_STATE_IMM:
-      ScoreBrackets->updateByEvent(SMEM_ACCESS, Inst);
+      Events.insert(SMEM_ACCESS);
       break;
     }
   }
+  return Events;
+}
 
-  if (!ST->hasWaitXcnt())
-    return;
+void SIInsertWaitcnts::updateEventWaitcntAfter(MachineInstr &Inst,
----------------
vporpo wrote:

It does need a comment (and many other places), but it would not be related to this change, so it's better to do this in a separate PR.

I agree, we should try to clean up the pass such that it becomes clear what changes state and what doesn't. This PR is a step towards this. Getting what events correspond to the instruction does not change any state so it can be moved into a separate function.

https://github.com/llvm/llvm-project/pull/180864