[PATCH] D72800: [MachineScheduler] Don't reorder when we can't cluster

[Jinsong Ji via Phabricator via llvm-commits]

jsji created this revision.
jsji added reviewers: foad, rampitec, fhahn, steven.zhang, atrick, MatzeB, arsenm.
Herald added subscribers: llvm-commits, javed.absar, hiraditya, wdng.
Herald added a project: LLVM.

https://reviews.llvm.org/D72706 tried to reduce reordering due to mem op
clustering. However, it introduce additional reordering when we can not
do mem op clustering.

This is breaking our downstream scheduler tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72800

Files:
  llvm/lib/CodeGen/MachineScheduler.cpp


Index: llvm/lib/CodeGen/MachineScheduler.cpp
===================================================================
--- llvm/lib/CodeGen/MachineScheduler.cpp
+++ llvm/lib/CodeGen/MachineScheduler.cpp
@@ -1573,26 +1573,28 @@
   for (unsigned Idx = 0, End = MemOpRecords.size(); Idx < (End - 1); ++Idx) {
     SUnit *SUa = MemOpRecords[Idx].SU;
     SUnit *SUb = MemOpRecords[Idx+1].SU;
-    if (SUa->NodeNum > SUb->NodeNum)
-      std::swap(SUa, SUb);
     if (TII->shouldClusterMemOps(*MemOpRecords[Idx].BaseOp,
                                  *MemOpRecords[Idx + 1].BaseOp,
-                                 ClusterLength) &&
-        DAG->addEdge(SUb, SDep(SUa, SDep::Cluster))) {
-      LLVM_DEBUG(dbgs() << "Cluster ld/st SU(" << SUa->NodeNum << ") - SU("
-                        << SUb->NodeNum << ")\n");
-      // Copy successor edges from SUa to SUb. Interleaving computation
-      // dependent on SUa can prevent load combining due to register reuse.
-      // Predecessor edges do not need to be copied from SUb to SUa since nearby
-      // loads should have effectively the same inputs.
-      for (const SDep &Succ : SUa->Succs) {
-        if (Succ.getSUnit() == SUb)
-          continue;
-        LLVM_DEBUG(dbgs() << "  Copy Succ SU(" << Succ.getSUnit()->NodeNum
-                          << ")\n");
-        DAG->addEdge(Succ.getSUnit(), SDep(SUb, SDep::Artificial));
-      }
-      ++ClusterLength;
+                                 ClusterLength)) {
+      if (SUa->NodeNum > SUb->NodeNum)
+        std::swap(SUa, SUb);
+      if (DAG->addEdge(SUb, SDep(SUa, SDep::Cluster))) {
+        LLVM_DEBUG(dbgs() << "Cluster ld/st SU(" << SUa->NodeNum << ") - SU("
+                          << SUb->NodeNum << ")\n");
+        // Copy successor edges from SUa to SUb. Interleaving computation
+        // dependent on SUa can prevent load combining due to register reuse.
+        // Predecessor edges do not need to be copied from SUb to SUa since
+        // nearby loads should have effectively the same inputs.
+        for (const SDep &Succ : SUa->Succs) {
+          if (Succ.getSUnit() == SUb)
+            continue;
+          LLVM_DEBUG(dbgs()
+                     << "  Copy Succ SU(" << Succ.getSUnit()->NodeNum << ")\n");
+          DAG->addEdge(Succ.getSUnit(), SDep(SUb, SDep::Artificial));
+        }
+        ++ClusterLength;
+      } else
+        ClusterLength = 1;
     } else
       ClusterLength = 1;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72800.238339.patch
Type: text/x-patch
Size: 2442 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200115/32b3f3c8/attachment.bin>