[PATCH] D132356: [SimplifyCFG] Don't widen cond br to cond br if false branch has successors

[Dmitry Makogon via Phabricator via llvm-commits]

dmakogon created this revision.
dmakogon added a reviewer: mkazantsev.
Herald added a subscriber: hiraditya.
Herald added a project: All.
dmakogon requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This fixes an infinite loop of transformations in SimplifyCFG (GItHub issue - https://github.com/llvm/llvm-project/issues/57221).
The problem is that there are two transforms in SimplifyCFG that do absolutely opposite things.
First, there is `tryWidenCondBranchToCondBranch` which does the following.
When it sees a branch on widenable condition followed by another conditional branch like here:

  bb:
    %tmp = call i1 @llvm.experimental.widenable.condition()
    br i1 %tmp, label %bb2, label %bb1
  
  bb1:                                              ; preds = %bb
    br i1 undef, label %bb7, label %bb5
  
  bb2:                                              ; preds = %bb
    br i1 undef, label %bb7, label %bb4

it thinks that it's profitable for bb2 to branch to bb1 instead of bb7 on true condition.
The IR after the transform:

  bb:
    %tmp = call i1 @llvm.experimental.widenable.condition()
    br i1 %tmp, label %bb2, label %bb1
  
  bb1:                                              ; preds = %bb
    br i1 undef, label %bb7, label %bb5
  
  bb2:                                              ; preds = %bb
    br i1 undef, label %bb1, label %bb4

Then, there is another transformation `SimplifyCondBranchToCondBranch`. It sees that bb2 branches to bb1 if the condition is true and that bb1 has the same condition. So instead of bb2->bb1->bb7 branches, it makes it just bb2->bb7 - the exact same IR before the first transform.

This patch limits the `tryWidenCondBranchToCondBranch` transform making it work only if the false block of widenable condition branch has no successors.
If that block has successors, then the possible impact of the transform is complicated. SimplifyCFG may apply other transforms after this one and produce unexpected results.
As far as I can see this transform only has impact when the no succesors condition stands.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132356

Files:
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/Transforms/SimplifyCFG/pr52290.ll


Index: llvm/test/Transforms/SimplifyCFG/pr52290.ll
===================================================================
--- llvm/test/Transforms/SimplifyCFG/pr52290.ll
+++ llvm/test/Transforms/SimplifyCFG/pr52290.ll
@@ -1,15 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -simplifycfg -S | FileCheck %s
 ; RUN: opt < %s -passes=simplifycfg -S | FileCheck %s
 
-; XFAIL: *
-; REQUIRES: asserts
-; FIXME: Fails due to infinite loop in iterativelySimplifyCFG.
-
 ; ModuleID = 'test/Transforms/SimplifyCFG/pr-new.ll'
 source_filename = "test/Transforms/SimplifyCFG/pr-new.ll"
 
 define i32 @test(float %arg) gc "statepoint-example" personality i32* ()* @blam {
-; CHECK-LABEL: @test
+; CHECK-LABEL: @test(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    [[TMP:%.*]] = call i1 @llvm.experimental.widenable.condition()
+; CHECK-NEXT:    br i1 [[TMP]], label [[BB2:%.*]], label [[BB1:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    br i1 undef, label [[BB7:%.*]], label [[BB5:%.*]]
+; CHECK:       bb2:
+; CHECK-NEXT:    [[TMP3:%.*]] = getelementptr inbounds i8, i8 addrspace(1)* undef, i64 16
+; CHECK-NEXT:    br i1 undef, label [[BB7]], label [[BB4:%.*]]
+; CHECK:       bb4:
+; CHECK-NEXT:    call void @snork() [ "deopt"() ]
+; CHECK-NEXT:    unreachable
+; CHECK:       bb5:
+; CHECK-NEXT:    ret i32 0
+; CHECK:       bb7:
+; CHECK-NEXT:    [[TMP8:%.*]] = call i32 (...) @llvm.experimental.deoptimize.i32(i32 10) [ "deopt"() ]
+; CHECK-NEXT:    ret i32 [[TMP8]]
+;
 bb:
   %tmp = call i1 @llvm.experimental.widenable.condition()
   br i1 %tmp, label %bb2, label %bb1
Index: llvm/lib/Transforms/Utils/SimplifyCFG.cpp
===================================================================
--- llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -4007,6 +4007,7 @@
   };
   if (BI->getSuccessor(1) != IfFalseBB && // no inf looping
       BI->getSuccessor(1)->getTerminatingDeoptimizeCall() && // profitability
+      llvm::succ_empty(IfFalseBB) &&
       NoSideEffects(*BI->getParent())) {
     auto *OldSuccessor = BI->getSuccessor(1);
     OldSuccessor->removePredecessor(BI->getParent());
@@ -4019,6 +4020,7 @@
   }
   if (BI->getSuccessor(0) != IfFalseBB && // no inf looping
       BI->getSuccessor(0)->getTerminatingDeoptimizeCall() && // profitability
+      llvm::succ_empty(IfFalseBB) &&
       NoSideEffects(*BI->getParent())) {
     auto *OldSuccessor = BI->getSuccessor(0);
     OldSuccessor->removePredecessor(BI->getParent());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D132356.454409.patch
Type: text/x-patch
Size: 2515 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220822/1dd9f9cd/attachment.bin>