[PATCH] D72031: [Scheduling] Create the missing dependency edges for store cluster

[qshanz via Phabricator via llvm-commits]

steven.zhang updated this revision to Diff 238202.
steven.zhang added a comment.

Address the comments. And all the case changes disappear with the patch https://reviews.llvm.org/D72706 landed. So, I didn't see the negative impact from this patch any more. And from the design, it is doing the right thing, without increasing the compiling time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72031

Files:
  llvm/lib/CodeGen/MachineScheduler.cpp


Index: llvm/lib/CodeGen/MachineScheduler.cpp
===================================================================
--- llvm/lib/CodeGen/MachineScheduler.cpp
+++ llvm/lib/CodeGen/MachineScheduler.cpp
@@ -1581,16 +1581,32 @@
         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));
+      if (IsLoad) {
+        // 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));
+        }
+      } else {
+        // Copy predecessor edges from SUb to SUa to avoid the SUnits that
+        // SUb dependent on scheduled in-between SUb and SUa. Successor edges
+        // do not need to be copied from SUa to SUb since no one will depend
+        // on stores.
+        // Notice that, we don't need to care about the memory dependency as
+        // we won't try to cluster them if they have any memory dependency.
+        for (const SDep &Pred : SUb->Preds) {
+          if (Pred.getSUnit() == SUa)
+            continue;
+          LLVM_DEBUG(dbgs()
+                     << "  Copy Pred SU(" << Pred.getSUnit()->NodeNum << ")\n");
+          DAG->addEdge(SUa, SDep(Pred.getSUnit(), SDep::Artificial));
+        }
       }
       ++ClusterLength;
     } else


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72031.238202.patch
Type: text/x-patch
Size: 2375 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200115/96aa9b96/attachment-0001.bin>