[PATCH] D18673: Fix PR26940 where compiles times regressed massively.

[Chandler Carruth via llvm-commits]

chandlerc created this revision.
chandlerc added a reviewer: echristo.
chandlerc added a subscriber: llvm-commits.
Herald added subscribers: mcrosier, MatzeB.

Patch by Jonas Paulsson. Original description:
Bugfix in buildSchedGraph() to make -dag-maps-huge-region work properly

I found that the reduction of the maps did in fact never happen in this
test case. This was because *all* the stores / loads were made with
addresses from arguments and they thus became "unknown" stores / loads.
Fixed by removing continue statements and making sure that the test for
reduction always takes place.

http://reviews.llvm.org/D18673

Files:
  lib/CodeGen/ScheduleDAGInstrs.cpp

Index: lib/CodeGen/ScheduleDAGInstrs.cpp
===================================================================
--- lib/CodeGen/ScheduleDAGInstrs.cpp
+++ lib/CodeGen/ScheduleDAGInstrs.cpp
@@ -1025,52 +1025,49 @@
 
         // Map this store to 'UnknownValue'.
         Stores.insert(SU, UnknownValue);
-        continue;
-      }
-
-      // Add precise dependencies against all previously seen memory
-      // accesses mapped to the same Value(s).
-      for (auto &underlObj : Objs) {
-        ValueType V = underlObj.getPointer();
-        bool ThisMayAlias = underlObj.getInt();
+      } else {
+        // Add precise dependencies against all previously seen memory
+        // accesses mapped to the same Value(s).
+        for (auto &underlObj : Objs) {
+          ValueType V = underlObj.getPointer();
+          bool ThisMayAlias = underlObj.getInt();
 
-        Value2SUsMap &stores_ = (ThisMayAlias ? Stores : NonAliasStores);
+          Value2SUsMap &stores_ = (ThisMayAlias ? Stores : NonAliasStores);
 
-        // Add dependencies to previous stores and loads mapped to V.
-        addChainDependencies(SU, stores_, V);
-        addChainDependencies(SU, (ThisMayAlias ? Loads : NonAliasLoads), V);
+          // Add dependencies to previous stores and loads mapped to V.
+          addChainDependencies(SU, stores_, V);
+          addChainDependencies(SU, (ThisMayAlias ? Loads : NonAliasLoads), V);
 
-        // Map this store to V.
-        stores_.insert(SU, V);
+          // Map this store to V.
+          stores_.insert(SU, V);
+        }
+        // The store may have dependencies to unanalyzable loads and
+        // stores.
+        addChainDependencies(SU, Loads, UnknownValue);
+        addChainDependencies(SU, Stores, UnknownValue);
       }
-      // The store may have dependencies to unanalyzable loads and
-      // stores.
-      addChainDependencies(SU, Loads, UnknownValue);
-      addChainDependencies(SU, Stores, UnknownValue);
-    }
-    else { // SU is a load.
+    } else { // SU is a load.
       if (Objs.empty()) {
         // An unknown load depends on all stores.
         addChainDependencies(SU, Stores);
         addChainDependencies(SU, NonAliasStores);
 
         Loads.insert(SU, UnknownValue);
-        continue;
-      }
+      } else {
+        for (auto &underlObj : Objs) {
+          ValueType V = underlObj.getPointer();
+          bool ThisMayAlias = underlObj.getInt();
 
-      for (auto &underlObj : Objs) {
-        ValueType V = underlObj.getPointer();
-        bool ThisMayAlias = underlObj.getInt();
+          // Add precise dependencies against all previously seen stores
+          // mapping to the same Value(s).
+          addChainDependencies(SU, (ThisMayAlias ? Stores : NonAliasStores), V);
 
-        // Add precise dependencies against all previously seen stores
-        // mapping to the same Value(s).
-        addChainDependencies(SU, (ThisMayAlias ? Stores : NonAliasStores), V);
-
-        // Map this load to V.
-        (ThisMayAlias ? Loads : NonAliasLoads).insert(SU, V);
+          // Map this load to V.
+          (ThisMayAlias ? Loads : NonAliasLoads).insert(SU, V);
+        }
+        // The load may have dependencies to unanalyzable stores.
+        addChainDependencies(SU, Stores, UnknownValue);
       }
-      // The load may have dependencies to unanalyzable stores.
-      addChainDependencies(SU, Stores, UnknownValue);
     }
 
     // Reduce maps if they grow huge.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D18673.52277.patch
Type: text/x-patch
Size: 3473 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160331/16ecb406/attachment.bin>