[PATCH] D34965: AMDGPU/SI: Force exports at the end for SI scheduler

[Axel Davy via Phabricator via llvm-commits]

axeldavy created this revision.
axeldavy added a project: AMDGPU.
Herald added subscribers: t-tye, tpr, dstuttard, yaxunl, nhaehnle, wdng, kzhuravl, arsenm, MatzeB.

Ideally exports should be done at the end of the shader for performance.
The position export could be done a bit earlier, but after all other memory operations.
This patch is a first step, and enforces all export to be at the end of the shader for the SI scheduler.


Repository:
  rL LLVM

https://reviews.llvm.org/D34965

Files:
  lib/Target/AMDGPU/SIMachineScheduler.cpp
  lib/Target/AMDGPU/SIMachineScheduler.h


Index: lib/Target/AMDGPU/SIMachineScheduler.h
===================================================================
--- lib/Target/AMDGPU/SIMachineScheduler.h
+++ lib/Target/AMDGPU/SIMachineScheduler.h
@@ -302,6 +302,9 @@
   // (we'd want these groups be at the end).
   void regroupNoUserInstructions();
 
+  // Give Reserved color to export instructions
+  void colorExports();
+
   void createBlocksForVariant(SISchedulerBlockCreatorVariant BlockVariant);
 
   void topologicalSort();
Index: lib/Target/AMDGPU/SIMachineScheduler.cpp
===================================================================
--- lib/Target/AMDGPU/SIMachineScheduler.cpp
+++ lib/Target/AMDGPU/SIMachineScheduler.cpp
@@ -1130,6 +1130,62 @@
   }
 }
 
+void SIScheduleBlockCreator::colorExports() {
+  unsigned ExportColor = NextNonReservedID++;
+  SmallVector<unsigned, 8> ExpGroup;
+
+  // Put all exports together in a block.
+  // The block will naturally end up being scheduled last,
+  // thus putting exports at the end of the schedule, which
+  // is better for performance.
+  // However we must ensure, for safety, the exports can be put
+  // together in the same block without any other instruction.
+  // This could happen, for example, when scheduling after regalloc
+  // if reloading a spilled register from memory using the same
+  // register than used in a previous export.
+  // If that happens, do not regroup the exports.
+  for (unsigned SUNum : DAG->TopDownIndex2SU) {
+    const SUnit &SU = DAG->SUnits[SUNum];
+    if (SIInstrInfo::isEXP(*SU.getInstr())) {
+      // Check the EXP can be added to the group safely,
+      // ie without needing any other instruction.
+      // The EXP is allowed to depend on other EXP
+      // (they will be in the same group).
+      for (unsigned j : ExpGroup) {
+        bool HasSubGraph;
+        std::vector<int> SubGraph;
+        // By construction (topological order), if SU and
+        // DAG->SUnits[j] are linked, DAG->SUnits[j] is neccessary
+        // in the parent graph of SU.
+#ifndef NDEBUG
+        SubGraph = DAG->GetTopo()->GetSubGraph(SU, DAG->SUnits[j],
+                                               HasSubGraph);
+        assert(!HasSubGraph);
+#endif
+        SubGraph = DAG->GetTopo()->GetSubGraph(DAG->SUnits[j], SU,
+                                               HasSubGraph);
+        if (!HasSubGraph)
+          continue; // No dependencies between each other
+
+        // SubGraph contains all the instructions required
+        // between EXP SUnits[j] and EXP SU.
+        for (unsigned k : SubGraph) {
+          if (!SIInstrInfo::isEXP(*DAG->SUnits[k].getInstr()))
+            // Other instructions than EXP would be required in the group.
+            // Abort the groupping.
+            return;
+        }
+      }
+
+      ExpGroup.push_back(SUNum);
+    }
+  }
+
+  // The group can be formed. Give the color.
+  for (unsigned j : ExpGroup)
+    CurrentColoring[j] = ExportColor;
+}
+
 void SIScheduleBlockCreator::createBlocksForVariant(SISchedulerBlockCreatorVariant BlockVariant) {
   unsigned DAGSize = DAG->SUnits.size();
   std::map<unsigned,unsigned> RealID;
@@ -1159,6 +1215,7 @@
   regroupNoUserInstructions();
   colorMergeConstantLoadsNextGroup();
   colorMergeIfPossibleNextGroupOnlyForReserved();
+  colorExports();
 
   // Put SUs of same color into same block
   Node2CurrentBlock.resize(DAGSize, -1);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D34965.105117.patch
Type: text/x-patch
Size: 3398 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170703/bb8b60f7/attachment.bin>