From 360ab707127d7f1718cf0fe0520b5a38ef207bc1 Mon Sep 17 00:00:00 2001 From: Max Kazantsev Date: Wed, 22 Jul 2020 13:14:50 +0700 Subject: [PATCH] [SimplifyCFG] Do not create unneeded PR Phi in block with convergent calls We do not thread blocks with convergent calls, but this check was missing when we decide to insert PR Phis into it (which we only do for threading). Differential Revision: https://reviews.llvm.org/D83936 Reviewed By: nikic --- clang/test/CodeGenOpenCL/convergent.cl | 5 ++--- llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 13 ++++++------- llvm/test/Transforms/SimplifyCFG/convergent.ll | 4 +--- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/clang/test/CodeGenOpenCL/convergent.cl b/clang/test/CodeGenOpenCL/convergent.cl index 631b04b5909c..f7d4cea8373d 100644 --- a/clang/test/CodeGenOpenCL/convergent.cl +++ b/clang/test/CodeGenOpenCL/convergent.cl @@ -70,10 +70,9 @@ void test_merge_if(int a) { // CHECK-NOT: call spir_func void @g() // CHECK: br label %[[if_end]] // CHECK: [[if_end]]: -// FIXME: SimplifyCFG is being stupid inserting this Phi. It is not supposed to be here. -// CHECK: %[[tobool_not_pr:.+]] = phi i1 +// CHECK-NOT: phi i1 // CHECK: tail call spir_func void @convfun() #[[attr4:.+]] -// CHECK: br i1 %[[tobool_not_pr]], label %[[if_end3:.+]], label %[[if_then2:.+]] +// CHECK: br i1 %[[tobool]], label %[[if_end3:.+]], label %[[if_then2:.+]] // CHECK: [[if_then2]]: // CHECK: tail call spir_func void @g() // CHECK: br label %[[if_end3:.+]] diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index cc6bf9e864d0..a578bbd3f65b 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -2233,6 +2233,12 @@ static bool BlockIsSimpleEnoughToThreadThrough(BasicBlock *BB) { for (Instruction &I : BB->instructionsWithoutDebug()) { if (Size > MaxSmallBlockSize) return false; // Don't clone large BB's. + + // Can't fold blocks that contain noduplicate or convergent calls. + if (CallInst *CI = dyn_cast(&I)) + if (CI->cannotDuplicate() || CI->isConvergent()) + return false; + // We will delete Phis while threading, so Phis should not be accounted in // block's size if (!isa(I)) @@ -2274,13 +2280,6 @@ static bool FoldCondBranchOnPHI(BranchInst *BI, const DataLayout &DL, if (!BlockIsSimpleEnoughToThreadThrough(BB)) return false; - // Can't fold blocks that contain noduplicate or convergent calls. - if (any_of(*BB, [](const Instruction &I) { - const CallInst *CI = dyn_cast(&I); - return CI && (CI->cannotDuplicate() || CI->isConvergent()); - })) - return false; - // Okay, this is a simple enough basic block. See if any phi values are // constants. for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) { diff --git a/llvm/test/Transforms/SimplifyCFG/convergent.ll b/llvm/test/Transforms/SimplifyCFG/convergent.ll index be8e8482e94b..1860f457da6c 100644 --- a/llvm/test/Transforms/SimplifyCFG/convergent.ll +++ b/llvm/test/Transforms/SimplifyCFG/convergent.ll @@ -4,7 +4,6 @@ declare void @foo() convergent -; FIXME: We should not be inserting a PR Phi here. define i32 @test_01(i32 %a) { ; CHECK-LABEL: @test_01( ; CHECK-NEXT: entry: @@ -14,9 +13,8 @@ define i32 @test_01(i32 %a) { ; CHECK-NEXT: call void @foo() ; CHECK-NEXT: br label [[MERGE]] ; CHECK: merge: -; CHECK-NEXT: [[COND_PR:%.*]] = phi i1 [ [[COND]], [[IF_FALSE]] ], [ true, [[ENTRY:%.*]] ] ; CHECK-NEXT: call void @foo() -; CHECK-NEXT: br i1 [[COND_PR]], label [[EXIT:%.*]], label [[IF_FALSE_2:%.*]] +; CHECK-NEXT: br i1 [[COND]], label [[EXIT:%.*]], label [[IF_FALSE_2:%.*]] ; CHECK: if.false.2: ; CHECK-NEXT: call void @foo() ; CHECK-NEXT: br label [[EXIT]]