[PATCH] D83936: [SimplifyCFG] Do not create unneeded PR Phi in block with convergent calls

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev created this revision.
mkazantsev added reviewers: lebedev.ri, vsk, davide, asbirlea, xbolva00.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

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).


https://reviews.llvm.org/D83936

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


Index: llvm/test/Transforms/SimplifyCFG/convergent.ll
===================================================================
--- llvm/test/Transforms/SimplifyCFG/convergent.ll
+++ 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 @@
 ; 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]]
Index: llvm/lib/Transforms/Utils/SimplifyCFG.cpp
===================================================================
--- llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2229,6 +2229,12 @@
   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))
@@ -2270,13 +2276,6 @@
   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) {
Index: clang/test/CodeGenOpenCL/convergent.cl
===================================================================
--- clang/test/CodeGenOpenCL/convergent.cl
+++ clang/test/CodeGenOpenCL/convergent.cl
@@ -70,10 +70,9 @@
 // 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:.+]]


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83936.278414.patch
Type: text/x-patch
Size: 2938 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200716/67fd3be6/attachment.bin>