From babae05237cb1914be0b2a25c56754ccc92084ee Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Tue, 4 Jun 2013 14:11:59 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@183226 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Utils/SimplifyCFG.cpp | 13 +++++++++---- test/Transforms/SimplifyCFG/PR16069.ll | 16 +++++++++++++++- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/Transforms/Utils/SimplifyCFG.cpp b/lib/Transforms/Utils/SimplifyCFG.cpp index 1b491c098cf..6d12f7a218d 100644 --- a/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/lib/Transforms/Utils/SimplifyCFG.cpp @@ -1537,18 +1537,23 @@ static bool SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB) { 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(ThenV); - if (!CE) + ConstantExpr *OrigCE = dyn_cast(OrigV); + ConstantExpr *ThenCE = dyn_cast(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 diff --git a/test/Transforms/SimplifyCFG/PR16069.ll b/test/Transforms/SimplifyCFG/PR16069.ll index 4e9f89660c4..0b3d6779451 100644 --- a/test/Transforms/SimplifyCFG/PR16069.ll +++ b/test/Transforms/SimplifyCFG/PR16069.ll @@ -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 +} -- 2.34.1