[PATCH] ScheduleDAGInstrs::buildSchedGraph() rewritten.

hfinkel at anl.gov hfinkel at anl.gov
Sun Apr 12 06:09:29 PDT 2015


Thanks again for working on this; I few minor comments below.

I ran through the test suite and self-hosting with this patch applied; all tests passed and I saw no significant performance changes, so I'm happy on that front!


================
Comment at: include/llvm/CodeGen/ScheduleDAGInstrs.h:149
@@ +148,3 @@
+    /// a huge scheduling region has been reduced by inserting a
+    /// barrier chain far below current instruction).
+    SUnit *BarrierChain;
----------------
This comment is confusing. "this SU" is that barrier chain?

================
Comment at: include/llvm/CodeGen/ScheduleDAGInstrs.h:176
@@ +175,3 @@
+      /// this operator w/ push_back().
+      ValueType &operator[](const SUList &Key) { assert(0 && "Don't use"); };
+
----------------
This needs to be:
  llvm_unreachable("Don't use");
(or, preferably, some more informative message)


================
Comment at: lib/CodeGen/ScheduleDAGInstrs.cpp:696
@@ +695,3 @@
+  // together hold this many SUs, a reduction of maps will be done.
+  const unsigned HugeRegion = 1000;
+
----------------
Please make this a cl::opt command-line parameter.

================
Comment at: lib/CodeGen/ScheduleDAGInstrs.cpp:700
@@ -776,1 +699,3 @@
+  // nodes at a time.
+  const unsigned ReductionSize = (HugeRegion / 2);
 
----------------
This too (a command-line parameter) -- having it default to HugeRegion/2 likely makes sense. (You can use the getNumOccurrences() function on a cl::opt to see if it has actually been set or not).

================
Comment at: lib/CodeGen/ScheduleDAGInstrs.cpp:896
@@ +895,3 @@
+    // Reduce maps if they grow huge.
+    if (Stores.size() + Loads.size() == HugeRegion) {
+      DEBUG(dbgs() << "Reducing Stores and Loads maps.\n";);
----------------
Should this be >=? We can add multiple dependencies at a time, so we might miss an exact threshold crossing?

http://reviews.llvm.org/D8705

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list