[llvm] AMDGPU: Fix temporal divergence introduced by machine-sink and performance regression introduced by D155343 (PR #67456)

Nicolai Hähnle via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 3 11:38:13 PDT 2023


================
@@ -171,6 +171,48 @@ bool SIInstrInfo::isIgnorableUse(const MachineOperand &MO) const {
          isVALU(*MO.getParent()) && !resultDependsOnExec(*MO.getParent());
 }
 
+bool SIInstrInfo::isSafeToSink(MachineInstr &MI,
+                               MachineBasicBlock *SuccToSinkTo,
+                               MachineCycleInfo *CI) const {
+  CI->clear();
+  CI->compute(*MI.getMF());
+  MachineRegisterInfo &MRI = MI.getMF()->getRegInfo();
+
+  // Check if sinking of MI would create temporal divergent use.
+  for (auto Op : MI.uses()) {
+    if (Op.isReg() && Op.getReg().isVirtual() &&
+        RI.isSGPRClass(MRI.getRegClass(Op.getReg()))) {
+      MachineInstr *SgprDef = MRI.getVRegDef(Op.getReg());
+
+      // SgprDef defined inside cycle
+      MachineCycle *FromCycle = CI->getCycle(SgprDef->getParent());
+      if (FromCycle == nullptr)
+        return true;
+
+      // After structurize-cfg, there should be exactly one cycle exit.
+      SmallVector<MachineBasicBlock *, 1> ExitBlocks;
+      FromCycle->getExitBlocks(ExitBlocks);
+      assert(ExitBlocks.size() == 1);
+      assert(ExitBlocks[0]->getSinglePredecessor());
+
+      // Cycle has divergent exit condition.
+      if (!hasDivergentBranch(ExitBlocks[0]->getSinglePredecessor()))
+        return true;
+
+      // SuccToSinkTo is not in the cycle.
+      if (FromCycle != CI->getCycle(SuccToSinkTo)) {
----------------
nhaehnle wrote:

This is a good start, but it doesn't yet quite capture the essence of what is happening. Consider the possible ways in which cycles may be nested. We can think of it as sinking MI successfully out of ever more cycles containing the SGPR def. It may be helpful to draw some diagrams on a piece of paper.

Suggestion:

* Get the cycle of `SuccToSinkTo` --> `ToCycle`.
* While `FromCycle` does not contain `ToCycle`:
  * If FromCycle has a divergent exit, return false: we've shown that sinking is unsafe
  * Otherwise, it is safe to sink out of FromCycle, but we need to continue checking: replace FromCycle by the parent cycle and repeat

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


More information about the llvm-commits mailing list