[PATCH] D127123: [AMDGPU] Add more expressive sched_barrier controls

Stanislav Mekhanoshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 6 11:55:07 PDT 2022


rampitec added a comment.

Any post-RA specific tests?



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMFMAIGroupLP.cpp:224
+  // Cache SchedGroups of each type if we have multiple SCHED_BARRIERs in a
+  // regeion.
+  //
----------------
Typo: regeion


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMFMAIGroupLP.cpp:257
   TII = ST.getInstrInfo();
   if (!ST.hasMAIInsts())
     return;
----------------
As is this is useful w/o MFMA too.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMFMAIGroupLP.cpp:264
 
-  const IsInstructionType isMFMAFn = [this](const MachineInstr &MI) {
-    if (TII->isMAI(MI) && MI.getOpcode() != AMDGPU::V_ACCVGPR_WRITE_B32_e64 &&
-        MI.getOpcode() != AMDGPU::V_ACCVGPR_READ_B32_e64) {
-      LLVM_DEBUG(dbgs() << "Found MFMA\n";);
-      return true;
+  LLVM_DEBUG(dbgs() << "Applying MFMAIGroupLPDAGMutation...\n");
+
----------------
... and it should not probably read 'MFMA' here too, just like as in the file and class name.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMFMAIGroupLP.cpp:412
+
+void SchedBarrierDAGMutation::resetSchedBarrierEdges(SUnit &SU) {
+  assert(SU.getInstr()->getOpcode() == AMDGPU::SCHED_BARRIER);
----------------
This sounds pretty dangerous, at least for non-artifical edges.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:2446
   (V_PERM_B32_e64  (i32 0), VSrc_b32:$a, (S_MOV_B32 (i32 0x02030001)))
 >;
 }
----------------
Drop it from the change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127123/new/

https://reviews.llvm.org/D127123



More information about the llvm-commits mailing list