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

[Adam Nemet via Phabricator via llvm-commits]

anemet updated this revision to Diff 100466.
anemet added a comment.

Removed the asserts.  As Danny put it:

"Given LLVM defines edge dominance in a way that means non-unique edges never dominate their end, this is a waste of time."

In other words, there is no need for an assert here since it's not the case
that the answer would be wrong or would take more time to compute than for
unique edges.  It's simply that the answer would always be non-dominance by
how domininance of edges is defined.

Also added a comment the explain the situation with non-unique edges.


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
@@ -150,12 +150,6 @@
 
 bool DominatorTree::dominates(const BasicBlockEdge &BBE,
                               const BasicBlock *UseBB) const {
-  // 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() &&
-         "This function is not efficient in handling multiple edges");
-
   // If the BB the edge ends in doesn't dominate the use BB, then the
   // edge also doesn't.
   const BasicBlock *Start = BBE.getStart();
@@ -201,12 +195,6 @@
 }
 
 bool DominatorTree::dominates(const BasicBlockEdge &BBE, const Use &U) const {
-  // 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() &&
-         "This function is not efficient in handling multiple edges");
-
   Instruction *UserInst = cast<Instruction>(U.getUser());
   // A PHI in the end of the edge is dominated by it.
   PHINode *PN = dyn_cast<PHINode>(UserInst);
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;
 };
 
@@ -143,6 +144,11 @@
   bool dominates(const Instruction *Def, const Use &U) const;
   bool dominates(const Instruction *Def, const Instruction *User) const;
   bool dominates(const Instruction *Def, const BasicBlock *BB) const;
+
+  /// Return true if an edge dominates a use.
+  ///
+  /// If BBE is not a unique edge between start and end of the edge, it can
+  /// never dominate the use.
   bool dominates(const BasicBlockEdge &BBE, const Use &U) const;
   bool dominates(const BasicBlockEdge &BBE, const BasicBlock *BB) const;
 


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