Reduce the number of dom queries made by GVN's conditional propagation

logic by half: isOnlyReachableViaThisEdge was trying to be clever and
handle the case of a branch to a basic block which is contained in a
loop.  This costs a domtree lookup and is completely useless due to
GVN's position in the pass pipeline: all loops have preheaders at this
point, which means it is enough for isOnlyReachableViaThisEdge to check
that Dst has only one predecessor.  (I checked this theoretical argument
by running over the entire nightly testsuite, and indeed it is so!).

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@149838 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp
index 03ebbb15124528c93092a18d23f4e52b4987edf5..70012524dc6e0aba8959df937ee806fb7be068d8 100644 (file)
--- a/lib/Transforms/Scalar/GVN.cpp
+++ b/lib/Transforms/Scalar/GVN.cpp
@@ -1994,37 +1994,15 @@ bool GVN::propagateEquality(Value *LHS, Value *RHS, BasicBlock *Root) {
 /// particular 'Dst' must not be reachable via another edge from 'Src'.
 static bool isOnlyReachableViaThisEdge(BasicBlock *Src, BasicBlock *Dst,
                                        DominatorTree *DT) {
-  // First off, there must not be more than one edge from Src to Dst, there
-  // should be exactly one.  So keep track of the number of times Src occurs
-  // as a predecessor of Dst and fail if it's more than once.
-  bool SawEdgeFromSrc = false;
-  for (pred_iterator PI = pred_begin(Dst), PE = pred_end(Dst); PI != PE; ++PI) {
-    if (*PI != Src)
-      continue;
-    // An edge from Src to Dst.
-    if (SawEdgeFromSrc)
-      // There are multiple edges from Src to Dst - fail.
-      return false;
-    SawEdgeFromSrc = true;
-  }
-  assert(SawEdgeFromSrc && "No edge between these basic blocks!");
-
-  // Secondly, any other predecessors of Dst should be dominated by Dst.  If the
-  // predecessor is not dominated by Dst, then it must be possible to reach it
-  // either without passing through Src (thus not via the edge) or by passing
-  // through Src but taking a different edge out of Src.  Either way Dst can be
-  // reached without passing via the edge, so fail.
-  for (pred_iterator PI = pred_begin(Dst), PE = pred_end(Dst); PI != PE; ++PI) {
-    BasicBlock *Pred = *PI;
-    if (Pred != Src && !DT->dominates(Dst, Pred))
-      return false;
-  }
-
-  // Every path from the entry block to Dst must at some point pass to Dst from
-  // a predecessor that is not dominated by Dst.  This predecessor can only be
-  // Src, since all others are dominated by Dst.  As there is only one edge from
-  // Src to Dst, the path passes by this edge.
-  return true;
+  // While in theory it is interesting to consider the case in which Dst has
+  // more than one predecessor, because Dst might be part of a loop which is
+  // only reachable from Src, in practice it is pointless since at the time
+  // GVN runs all such loops have preheaders, which means that Dst will have
+  // been changed to have only one predecessor, namely Src.
+  pred_iterator PI = pred_begin(Dst), PE = pred_end(Dst);
+  assert(PI != PE && *PI == Src && "No edge between these basic blocks!");
+  (void)Src;
+  return PE == ++PI;
 }
 
 /// processInstruction - When calculating availability, handle an instruction

diff --git a/test/Transforms/GVN/condprop.ll b/test/Transforms/GVN/condprop.ll
index 0b31b01b7b1432bbcaaf69faf1e8cfd54221fb7c..c17c994011eb0a522fd5861bb73518f89223fbf8 100644 (file)
--- a/test/Transforms/GVN/condprop.ll
+++ b/test/Transforms/GVN/condprop.ll
@@ -55,25 +55,6 @@ return:              ; preds = %bb8
 }
 
 declare void @foo(i1)
-
-; CHECK: @test2
-define void @test2(i1 %x, i1 %y) {
-  %z = or i1 %x, %y
-  br i1 %z, label %true, label %false
-true:
-; CHECK: true:
-  %z2 = or i1 %x, %y
-  call void @foo(i1 %z2)
-; CHECK: call void @foo(i1 true)
-  br label %true
-false:
-; CHECK: false:
-  %z3 = or i1 %x, %y
-  call void @foo(i1 %z3)
-; CHECK: call void @foo(i1 false)
-  br label %false
-}
-
 declare void @bar(i32)
 
 ; CHECK: @test3