[PATCH] D45647: Limit the MaxSteps used in hasPredessorHelper

[Tom Rix via Phabricator via llvm-commits]

trixirt created this revision.
trixirt added reviewers: hans, bogner.
Herald added a subscriber: llvm-commits.

hasPredecessorHelper uses SmallPtrSet for the Visted data structure.
SmallPtrSet assumes it's size is less than 32 but can grow as large
as there is memory.  When it does grow, the rehashing of it's large
table is very expensive.

By instrumenting hasPredecessorHelp and printing at the final Visited
size for a llvm+clang build, it was seen that nearly all of the
Visited's were less than 16.

In a 'small' 2000 line test case, when the Visited size was 1200,
the compile time 2 minutes.  Now is down to 1 sec.

Only the function causing the problem was changed.
If anyone is interested in the instrumented data / histograms contact me offline.


Repository:
  rL LLVM

https://reviews.llvm.org/D45647

Files:
  include/llvm/CodeGen/SelectionDAGNodes.h
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp


Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -13251,7 +13251,6 @@
 
   SmallPtrSet<const SDNode *, 16> Visited;
   SmallVector<const SDNode *, 8> Worklist;
-  unsigned int Max = 8192;
   // Search Ops of store candidates.
   for (unsigned i = 0; i < NumStores; ++i) {
     SDNode *n = StoreNodes[i].MemNode;
@@ -13262,7 +13261,7 @@
   // Search through DAG. We can stop early if we find a store node.
   for (unsigned i = 0; i < NumStores; ++i)
     if (SDNode::hasPredecessorHelper(StoreNodes[i].MemNode, Visited, Worklist,
-                                     Max))
+                                     SDNode::hasPredecessorHelperMaxSteps()))
       return false;
   return true;
 }
Index: include/llvm/CodeGen/SelectionDAGNodes.h
===================================================================
--- include/llvm/CodeGen/SelectionDAGNodes.h
+++ include/llvm/CodeGen/SelectionDAGNodes.h
@@ -797,6 +797,31 @@
   /// NOTE: This is an expensive method. Use it carefully.
   bool hasPredecessor(const SDNode *N) const;
 
+  /// Return a value to use in the MaxSteps of hasPredecessorHelper
+  /// This value helps to enforce the assumption that SmallPtrSet is small
+  /// When SmallPtrSet is large, rehashing Visited is the performace
+  /// bottleneck of the compiler.
+  static unsigned int hasPredecessorHelperMaxSteps(unsigned int OptimizationLevel = 2) {
+    //
+    // From reviewing the final Visited size while building llvm+clang,
+    // the mean was 12.45 and the standard deviation was 11.73.
+    // For SmallPtrSet, SmallSize max is 32.
+    unsigned int ret = 32;
+    // The maximum MaxSteps currently in use is 8192
+    // Do not go higher.
+    unsigned int max = 8192;
+    if (OptimizationLevel > 9) {
+      ret = max;
+    } else {
+      ret <<= OptimizationLevel;
+    }
+    // SmallPtrSet rehashes on powers of two.
+    // Reduce the MaxSteps by a bit to stop the last rehash.
+    unsigned int abit = 4;
+    ret -= abit;
+    return ret;
+  }
+
   /// Returns true if N is a predecessor of any node in Worklist. This
   /// helper keeps Visited and Worklist sets externally to allow unions
   /// searches to be performed in parallel, caching of results across


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45647.142491.patch
Type: text/x-patch
Size: 2360 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180413/bb890e63/attachment.bin>