[PATCH] D33584: Remove a quadratic behavior in assert-enabled builds

[Adam Nemet via Phabricator via llvm-commits]

anemet created this revision.

GVN propagates the equivalence from a condition into the blocks guarded by the
condition.  E.g. for 'if (a == 7) { ... }', 'a' will be replaced in the block
with 7.  It does this by replacing all the uses of 'a' that are dominated by
the true edge.

For a switch with N cases and U uses of the value, this will mean N * U calls
to dominates.  Asserting isSingleEdge in dominates make this N^2 * U because
this function checks for the uniqueness of the edge. I.e. traverses each edge
between the SwitchInst's block and the cases.

This moves the full check under EXPENSIVE_CHECKS and only asserts isSingleEdge
if there are less then 50 edges to check.  This brings build time down by an
order of magnitude for an input that has ~10k cases in a switch statement.


https://reviews.llvm.org/D33584

Files:
  include/llvm/IR/Dominators.h
  lib/IR/Dominators.cpp


Index: lib/IR/Dominators.cpp
===================================================================
--- lib/IR/Dominators.cpp
+++ lib/IR/Dominators.cpp
@@ -120,6 +120,21 @@
   return &*I == Def;
 }
 
+/// \brief A cheaper version of linear BasicBlockEdge::isSingleEdge that may
+/// report a single edge for a SwitchInst even though there are actually
+/// multiple edges.
+static bool maybeSingleEdge(const BasicBlockEdge &BBE) {
+#ifdef EXPENSIVE_CHECKS
+  bool CheckSingleEdge = true;
+#else
+  // This removes a noticeable quadratic behavior with asserts enabled when GVN
+  // propagates equality for a switch statement with a large number of cases.
+  bool CheckSingleEdge =
+      BBE.getStart()->getTerminator()->getNumSuccessors() < 50;
+#endif
+  return !CheckSingleEdge || BBE.isSingleEdge();
+}
+
 // true if Def would dominate a use in any instruction in UseBB.
 // note that dominates(Def, Def->getParent()) is false.
 bool DominatorTree::dominates(const Instruction *Def,
@@ -153,7 +168,7 @@
   // Assert that we have a single edge. We could handle them by simply
   // returning false, but since isSingleEdge is linear on the number of
   // edges, the callers can normally handle them more efficiently.
-  assert(BBE.isSingleEdge() &&
+  assert(maybeSingleEdge(BBE) &&
          "This function is not efficient in handling multiple edges");
 
   // If the BB the edge ends in doesn't dominate the use BB, then the
@@ -204,7 +219,7 @@
   // Assert that we have a single edge. We could handle them by simply
   // returning false, but since isSingleEdge is linear on the number of
   // edges, the callers can normally handle them more efficiently.
-  assert(BBE.isSingleEdge() &&
+  assert(maybeSingleEdge(BBE) &&
          "This function is not efficient in handling multiple edges");
 
   Instruction *UserInst = cast<Instruction>(U.getUser());
Index: include/llvm/IR/Dominators.h
===================================================================
--- include/llvm/IR/Dominators.h
+++ include/llvm/IR/Dominators.h
@@ -66,6 +66,7 @@
     return End;
   }
 
+  /// Check if this is the only edge between Start and End.
   bool isSingleEdge() const;
 };
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33584.100363.patch
Type: text/x-patch
Size: 2169 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170526/3b191a5f/attachment.bin>