[llvm-commits] [llvm] r149838 - in /llvm/trunk: lib/Transforms/Scalar/GVN.cpp test/Transforms/GVN/condprop.ll

[Duncan Sands]

Author: baldrick
Date: Sun Feb  5 12:25:50 2012
New Revision: 149838

URL: http://llvm.org/viewvc/llvm-project?rev=149838&view=rev
Log:
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!).

Modified:
    llvm/trunk/lib/Transforms/Scalar/GVN.cpp
    llvm/trunk/test/Transforms/GVN/condprop.ll

Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=149838&r1=149837&r2=149838&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Sun Feb  5 12:25:50 2012
@@ -1994,37 +1994,15 @@
 /// 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

Modified: llvm/trunk/test/Transforms/GVN/condprop.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/condprop.ll?rev=149838&r1=149837&r2=149838&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/GVN/condprop.ll (original)
+++ llvm/trunk/test/Transforms/GVN/condprop.ll Sun Feb  5 12:25:50 2012
@@ -55,25 +55,6 @@
 }
 
 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