[clang] 360ab70 - [SimplifyCFG] Do not create unneeded PR Phi in block with convergent calls

[Max Kazantsev via cfe-commits]

Author: Max Kazantsev
Date: 2020-07-22T13:53:50+07:00
New Revision: 360ab707127d7f1718cf0fe0520b5a38ef207bc1

URL: https://github.com/llvm/llvm-project/commit/360ab707127d7f1718cf0fe0520b5a38ef207bc1
DIFF: https://github.com/llvm/llvm-project/commit/360ab707127d7f1718cf0fe0520b5a38ef207bc1.diff

LOG: [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

Added: 
    

Modified: 
    clang/test/CodeGenOpenCL/convergent.cl
    llvm/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/test/Transforms/SimplifyCFG/convergent.ll

Removed: 
    


################################################################################
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<CallInst>(&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<PHINode>(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<CallInst>(&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]]