[llvm] r183226 - Second part of pr16069

[Rafael Espindola]

Author: rafael
Date: Tue Jun  4 09:11:59 2013
New Revision: 183226

URL: http://llvm.org/viewvc/llvm-project?rev=183226&view=rev
Log:
Second part of pr16069

The problem this time seems to be a thinko. We were assuming that in the CFG

A
| \
|  B
| /
C

speculating the basic block B would cause only the phi value for the B->C edge
to be speculated. That is not true, the phi's are semantically in the edges, so
if the A->B->C path is taken, any code needed for A->C is not executed and we
have to consider it too when deciding to speculate B.

Modified:
    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/trunk/test/Transforms/SimplifyCFG/PR16069.ll

Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=183226&r1=183225&r2=183226&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Tue Jun  4 09:11:59 2013
@@ -1537,18 +1537,23 @@ static bool SpeculativelyExecuteBB(Branc
     Value *OrigV = PN->getIncomingValueForBlock(BB);
     Value *ThenV = PN->getIncomingValueForBlock(ThenBB);
 
+    // FIXME: Try to remove some of the duplication with HoistThenElseCodeToIf.
     // Skip PHIs which are trivial.
     if (ThenV == OrigV)
       continue;
 
     HaveRewritablePHIs = true;
-    ConstantExpr *CE = dyn_cast<ConstantExpr>(ThenV);
-    if (!CE)
+    ConstantExpr *OrigCE = dyn_cast<ConstantExpr>(OrigV);
+    ConstantExpr *ThenCE = dyn_cast<ConstantExpr>(ThenV);
+    if (!OrigCE && !ThenCE)
       continue; // Known safe and cheap.
 
-    if (!isSafeToSpeculativelyExecute(CE))
+    if ((ThenCE && !isSafeToSpeculativelyExecute(ThenCE)) ||
+        (OrigCE && !isSafeToSpeculativelyExecute(OrigCE)))
       return false;
-    if (ComputeSpeculationCost(CE) > PHINodeFoldingThreshold)
+    unsigned OrigCost = OrigCE ? ComputeSpeculationCost(OrigCE) : 0;
+    unsigned ThenCost = ThenCE ? ComputeSpeculationCost(ThenCE) : 0;
+    if (OrigCost + ThenCost > 2 * PHINodeFoldingThreshold)
       return false;
 
     // Account for the cost of an unfolded ConstantExpr which could end up

Modified: llvm/trunk/test/Transforms/SimplifyCFG/PR16069.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/PR16069.ll?rev=183226&r1=183225&r2=183226&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/SimplifyCFG/PR16069.ll (original)
+++ llvm/trunk/test/Transforms/SimplifyCFG/PR16069.ll Tue Jun  4 09:11:59 2013
@@ -1,8 +1,9 @@
 ; RUN: opt < %s -simplifycfg -S | FileCheck %s
 
-; CHECK-NOT: select
 @b = extern_weak global i32
+
 define i32 @foo(i1 %y) {
+; CHECK: define i32 @foo(i1 %y) {
   br i1 %y, label %bb1, label %bb2
 bb1:
   br label %bb3
@@ -10,5 +11,18 @@ bb2:
   br label %bb3
 bb3:
   %cond.i = phi i32 [ 0, %bb1 ], [ srem (i32 1, i32 zext (i1 icmp eq (i32* @b, i32* null) to i32)), %bb2 ]
+; CHECK: phi i32 {{.*}} srem (i32 1, i32 zext (i1 icmp eq (i32* @b, i32* null) to i32)), %bb2
   ret i32 %cond.i
 }
+
+define i32 @foo2(i1 %x) {
+; CHECK: define i32 @foo2(i1 %x) {
+bb0:
+  br i1 %x, label %bb1, label %bb2
+bb1:
+  br label %bb2
+bb2:
+  %cond = phi i32 [ 0, %bb1 ], [ srem (i32 1, i32 zext (i1 icmp eq (i32* @b, i32* null) to i32)), %bb0 ]
+; CHECK:  %cond = phi i32 [ 0, %bb1 ], [ srem (i32 1, i32 zext (i1 icmp eq (i32* @b, i32* null) to i32)), %bb0 ]
+  ret i32 %cond
+}