[llvm] a4772cb - Revert "[SimplifyCFG] Thread branches on same condition in more cases (PR54980)"

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 5 07:57:56 PDT 2022


Author: Nikita Popov
Date: 2022-07-05T16:57:46+02:00
New Revision: a4772cbaf0dc717ab6b4639272ca2910897613f0

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

LOG: Revert "[SimplifyCFG] Thread branches on same condition in more cases (PR54980)"

This reverts commit 4e545bdb355a470d601e9bb7f7b2693c99e61a3e.

The newly added test is the third infinite combine loop caused by
this change. In this case, it's a combination of the branch to
common dest and jump threading folds that keeps peeling off loop
iterations.

The core problem here is that we ideally would not thread over
loop backedges, both because it is potentially non-profitable
(it may break canonical loop structure) and because it may result
in these kinds of loops. Unfortunately, due to the lack of a
dominator tree in SimplifyCFG, there is no good way to prevent
this. While we have LoopHeaders, this is an optional structure and
we don't do a good job of keeping it up to date. It would be fine
for a profitability check, but is not suitable for a correctness
check.

So for now I'm just giving up here, as I don't see a good way to
robustly prevent infinite combine loops.

Fixes https://github.com/llvm/llvm-project/issues/56203.

Added: 
    

Modified: 
    clang/test/CodeGenObjC/exceptions.m
    clang/test/CodeGenObjCXX/exceptions-legacy.mm
    llvm/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/test/CodeGen/AArch64/arm64-andCmpBrToTBZ.ll
    llvm/test/Transforms/GVNSink/sink-common-code.ll
    llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
    llvm/test/Transforms/SimplifyCFG/jump-threading.ll
    llvm/test/Transforms/SimplifyCFG/pr55765.ll
    llvm/test/Transforms/SimplifyCFG/wc-widen-block.ll

Removed: 
    


################################################################################
diff  --git a/clang/test/CodeGenObjC/exceptions.m b/clang/test/CodeGenObjC/exceptions.m
index 302e8af51ed8e..e01965edd73f2 100644
--- a/clang/test/CodeGenObjC/exceptions.m
+++ b/clang/test/CodeGenObjC/exceptions.m
@@ -25,12 +25,11 @@ void f1(void) {
     // CHECK-NEXT: icmp
     // CHECK-NEXT: br i1
     @try {
+    // CHECK:      call void asm sideeffect "", "=*m"
     // CHECK:      call void asm sideeffect "", "*m"
     // CHECK-NEXT: call void @foo()
       foo();
     // CHECK:      call void @objc_exception_try_exit
-    // CHECK:      try.handler:
-    // CHECK:      call void asm sideeffect "", "=*m"
 
     } @finally {
       break;
@@ -54,6 +53,12 @@ int f2(void) {
   // CHECK-NEXT:   [[CAUGHT:%.*]] = icmp eq i32 [[SETJMP]], 0
   // CHECK-NEXT:   br i1 [[CAUGHT]]
   @try {
+    // Landing pad.  Note that we elide the re-enter.
+    // CHECK:      call void asm sideeffect "", "=*m,=*m"(i32* nonnull elementtype(i32) [[X]]
+    // CHECK-NEXT: call i8* @objc_exception_extract
+    // CHECK-NEXT: [[T1:%.*]] = load i32, i32* [[X]]
+    // CHECK-NEXT: [[T2:%.*]] = add nsw i32 [[T1]], -1
+
     // CHECK: store i32 6, i32* [[X]]
     x++;
     // CHECK-NEXT: call void asm sideeffect "", "*m,*m"(i32* nonnull elementtype(i32) [[X]]
@@ -62,12 +67,6 @@ int f2(void) {
     // CHECK-NEXT: [[T:%.*]] = load i32, i32* [[X]]
     foo();
   } @catch (id) {
-    // Landing pad.  Note that we elide the re-enter.
-    // CHECK:      call void asm sideeffect "", "=*m,=*m"(i32* nonnull elementtype(i32) [[X]]
-    // CHECK-NEXT: call i8* @objc_exception_extract
-    // CHECK-NEXT: [[T1:%.*]] = load i32, i32* [[X]]
-    // CHECK-NEXT: [[T2:%.*]] = add nsw i32 [[T1]], -1
-
     x--;
   }
 

diff  --git a/clang/test/CodeGenObjCXX/exceptions-legacy.mm b/clang/test/CodeGenObjCXX/exceptions-legacy.mm
index 4166f635deecf..9d9e4e4b86df6 100644
--- a/clang/test/CodeGenObjCXX/exceptions-legacy.mm
+++ b/clang/test/CodeGenObjCXX/exceptions-legacy.mm
@@ -63,19 +63,20 @@ void test1(id obj, bool *failed) {
 //   Body.
 // CHECK:      invoke void @_Z3foov()
 
+//   Catch handler.  Reload of 'failed' address is unnecessary.
+// CHECK:      [[T0:%.*]] = load i8*, i8**
+// CHECK-NEXT: store i8 1, i8* [[T0]],
+// CHECK-NEXT: br label
+
 //   Leave the @try.
 // CHECK:      call void @objc_exception_try_exit([[BUF_T]]* nonnull [[BUF]])
 // CHECK-NEXT: br label
 // CHECK:      ret void
 
+
 //   Real EH cleanup.
 // CHECK:      [[T0:%.*]] = landingpad
 // CHECK-NEXT:    cleanup
 // CHECK-NEXT: call void @objc_exception_try_exit([[BUF_T]]* nonnull [[BUF]])
 // CHECK-NEXT: resume
 
-//   Catch handler.  Reload of 'failed' address is unnecessary.
-// CHECK:      [[T0:%.*]] = load i8*, i8**
-// CHECK-NEXT: store i8 1, i8* [[T0]],
-// CHECK-NEXT: br label
-

diff  --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index bbf596e1a1405..6b0f1e3760960 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2976,10 +2976,8 @@ static bool BlockIsSimpleEnoughToThreadThrough(BasicBlock *BB) {
   return true;
 }
 
-static ConstantInt *
-getKnownValueOnEdge(Value *V, BasicBlock *From, BasicBlock *To,
-                    SmallDenseMap<std::pair<BasicBlock *, BasicBlock *>,
-                                  ConstantInt *> &Visited) {
+static ConstantInt *getKnownValueOnEdge(Value *V, BasicBlock *From,
+                                        BasicBlock *To) {
   // Don't look past the block defining the value, we might get the value from
   // a previous loop iteration.
   auto *I = dyn_cast<Instruction>(V);
@@ -2993,23 +2991,7 @@ getKnownValueOnEdge(Value *V, BasicBlock *From, BasicBlock *To,
     return BI->getSuccessor(0) == To ? ConstantInt::getTrue(BI->getContext())
                                      : ConstantInt::getFalse(BI->getContext());
 
-  // Limit the amount of blocks we inspect.
-  if (Visited.size() >= 8)
-    return nullptr;
-
-  auto Pair = Visited.try_emplace({From, To}, nullptr);
-  if (!Pair.second)
-    return Pair.first->second;
-
-  // Check whether the known value is the same for all predecessors.
-  ConstantInt *Common = nullptr;
-  for (BasicBlock *Pred : predecessors(From)) {
-    ConstantInt *C = getKnownValueOnEdge(V, Pred, From, Visited);
-    if (!C || (Common && Common != C))
-      return nullptr;
-    Common = C;
-  }
-  return Visited[{From, To}] = Common;
+  return nullptr;
 }
 
 /// If we have a conditional branch on something for which we know the constant
@@ -3034,9 +3016,8 @@ FoldCondBranchOnValueKnownInPredecessorImpl(BranchInst *BI, DomTreeUpdater *DTU,
       if (auto *CB = dyn_cast<ConstantInt>(U))
         KnownValues[CB].insert(PN->getIncomingBlock(U));
   } else {
-    SmallDenseMap<std::pair<BasicBlock *, BasicBlock *>, ConstantInt *> Visited;
     for (BasicBlock *Pred : predecessors(BB)) {
-      if (ConstantInt *CB = getKnownValueOnEdge(Cond, Pred, BB, Visited))
+      if (ConstantInt *CB = getKnownValueOnEdge(Cond, Pred, BB))
         KnownValues[CB].insert(Pred);
     }
   }

diff  --git a/llvm/test/CodeGen/AArch64/arm64-andCmpBrToTBZ.ll b/llvm/test/CodeGen/AArch64/arm64-andCmpBrToTBZ.ll
index 4879d2e26aebf..f528c9cfabf4c 100644
--- a/llvm/test/CodeGen/AArch64/arm64-andCmpBrToTBZ.ll
+++ b/llvm/test/CodeGen/AArch64/arm64-andCmpBrToTBZ.ll
@@ -36,7 +36,7 @@ if.then3:                                         ; preds = %_ZNK7WebCore4Node10
 if.end5:                                          ; preds = %_ZNK7WebCore4Node10hasTagNameERKNS_13QualifiedNameE.exit, %lor.rhs.i.i.i
 ; CHECK: %if.end5
 ; CHECK: tbz
-  br i1 %IsEditable, label %if.end12, label %land.rhs.i19, !prof !1
+  br i1 %tobool.i.i.i, label %if.end12, label %land.rhs.i19, !prof !1
 
 land.rhs.i19:                                     ; preds = %if.end5
   %cmp.i.i.i18 = icmp eq i8* %str6, %str7

diff  --git a/llvm/test/Transforms/GVNSink/sink-common-code.ll b/llvm/test/Transforms/GVNSink/sink-common-code.ll
index 32f9f07dbc8b7..65adca157ab89 100644
--- a/llvm/test/Transforms/GVNSink/sink-common-code.ll
+++ b/llvm/test/Transforms/GVNSink/sink-common-code.ll
@@ -604,7 +604,7 @@ succ:
 declare void @g()
 
 ; CHECK-LABEL: test_pr30292
-; CHECK: phi i32 [ 0, %entry ], [ %add1, %succ ]
+; CHECK: phi i32 [ 0, %entry ], [ %add1, %succ ], [ %add2, %two ]
 
 define zeroext i1 @test_pr30244(i1 zeroext %flag, i1 zeroext %flag2, i32 %blksA, i32 %blksB, i32 %nblks) {
 

diff  --git a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
index f066c672ad4ba..2ac555ca0caaa 100644
--- a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
+++ b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
@@ -885,9 +885,9 @@ define void @test_pr30292(i1 %cond, i1 %cond2, i32 %a, i32 %b) {
 ; CHECK:       two:
 ; CHECK-NEXT:    call void @g()
 ; CHECK-NEXT:    [[ADD2:%.*]] = add i32 [[A]], 1
-; CHECK-NEXT:    br label [[TWO:%.*]]
+; CHECK-NEXT:    br label [[SUCC]]
 ; CHECK:       succ:
-; CHECK-NEXT:    [[P:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[ADD1]], [[SUCC]] ]
+; CHECK-NEXT:    [[P:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[ADD1]], [[SUCC]] ], [ [[ADD2]], [[TWO:%.*]] ]
 ; CHECK-NEXT:    br i1 [[COND:%.*]], label [[TWO]], label [[SUCC]]
 ;
 entry:

diff  --git a/llvm/test/Transforms/SimplifyCFG/jump-threading.ll b/llvm/test/Transforms/SimplifyCFG/jump-threading.ll
index aaebd0d5d4311..4f99e55b15a3b 100644
--- a/llvm/test/Transforms/SimplifyCFG/jump-threading.ll
+++ b/llvm/test/Transforms/SimplifyCFG/jump-threading.ll
@@ -145,10 +145,16 @@ define void @test_same_cond_simple(i1 %c) {
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
 ; CHECK:       if:
 ; CHECK-NEXT:    call void @foo()
-; CHECK-NEXT:    call void @foo()
-; CHECK-NEXT:    br label [[JOIN2:%.*]]
+; CHECK-NEXT:    br label [[JOIN:%.*]]
 ; CHECK:       else:
 ; CHECK-NEXT:    call void @bar()
+; CHECK-NEXT:    br label [[JOIN]]
+; CHECK:       join:
+; CHECK-NEXT:    br i1 [[C]], label [[IF2:%.*]], label [[ELSE2:%.*]]
+; CHECK:       if2:
+; CHECK-NEXT:    call void @foo()
+; CHECK-NEXT:    br label [[JOIN2:%.*]]
+; CHECK:       else2:
 ; CHECK-NEXT:    call void @bar()
 ; CHECK-NEXT:    br label [[JOIN2]]
 ; CHECK:       join2:
@@ -184,12 +190,17 @@ define void @test_same_cond_extra_use(i1 %c) {
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
 ; CHECK:       if:
 ; CHECK-NEXT:    call void @foo()
-; CHECK-NEXT:    call void @use.i1(i1 true)
-; CHECK-NEXT:    call void @foo()
-; CHECK-NEXT:    br label [[JOIN2:%.*]]
+; CHECK-NEXT:    br label [[JOIN:%.*]]
 ; CHECK:       else:
 ; CHECK-NEXT:    call void @bar()
-; CHECK-NEXT:    call void @use.i1(i1 false)
+; CHECK-NEXT:    br label [[JOIN]]
+; CHECK:       join:
+; CHECK-NEXT:    call void @use.i1(i1 [[C]])
+; CHECK-NEXT:    br i1 [[C]], label [[IF2:%.*]], label [[ELSE2:%.*]]
+; CHECK:       if2:
+; CHECK-NEXT:    call void @foo()
+; CHECK-NEXT:    br label [[JOIN2:%.*]]
+; CHECK:       else2:
 ; CHECK-NEXT:    call void @bar()
 ; CHECK-NEXT:    br label [[JOIN2]]
 ; CHECK:       join2:
@@ -226,11 +237,17 @@ define void @test_same_cond_extra_use_
diff erent_block(i1 %c) {
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
 ; CHECK:       if:
 ; CHECK-NEXT:    call void @foo()
+; CHECK-NEXT:    br label [[JOIN:%.*]]
+; CHECK:       else:
+; CHECK-NEXT:    call void @bar()
+; CHECK-NEXT:    br label [[JOIN]]
+; CHECK:       join:
+; CHECK-NEXT:    br i1 [[C]], label [[IF2:%.*]], label [[ELSE2:%.*]]
+; CHECK:       if2:
 ; CHECK-NEXT:    call void @use.i1(i1 [[C]])
 ; CHECK-NEXT:    call void @foo()
 ; CHECK-NEXT:    br label [[JOIN2:%.*]]
-; CHECK:       else:
-; CHECK-NEXT:    call void @bar()
+; CHECK:       else2:
 ; CHECK-NEXT:    call void @use.i1(i1 [[C]])
 ; CHECK-NEXT:    call void @bar()
 ; CHECK-NEXT:    br label [[JOIN2]]
@@ -320,7 +337,7 @@ define void @infloop(i1 %cmp.a, i1 %cmp.b, i1 %cmp.c) {
 ; CHECK:       while.body:
 ; CHECK-NEXT:    br i1 [[CMP_C:%.*]], label [[C_EXIT:%.*]], label [[LAND:%.*]]
 ; CHECK:       while.body.thread:
-; CHECK-NEXT:    br i1 [[CMP_C]], label [[WHILE_BODY_THREAD]], label [[LAND]]
+; CHECK-NEXT:    br i1 [[CMP_C]], label [[WHILE_COND]], label [[LAND]]
 ; CHECK:       land:
 ; CHECK-NEXT:    tail call void @bar()
 ; CHECK-NEXT:    br label [[WHILE_COND]]
@@ -358,3 +375,48 @@ c.exit:                                           ; preds = %while.body
 for.d:                                            ; preds = %c.exit
   ret void
 }
+
+; A combination of "branch to common dest" and jump threading kept peeling
+; off loop iterations here.
+
+define void @infloop_pr56203(i1 %c1, i1 %c2) {
+; CHECK-LABEL: @infloop_pr56203(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br i1 [[C1:%.*]], label [[EXIT:%.*]], label [[IF:%.*]]
+; CHECK:       if:
+; CHECK-NEXT:    call void @foo()
+; CHECK-NEXT:    [[C3:%.*]] = icmp eq i64 0, 0
+; CHECK-NEXT:    [[OR_COND:%.*]] = or i1 [[C2:%.*]], [[C3]]
+; CHECK-NEXT:    br i1 [[OR_COND]], label [[EXIT]], label [[LOOP_SPLIT:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[C3_OLD:%.*]] = icmp eq i64 0, 0
+; CHECK-NEXT:    br i1 [[C3_OLD]], label [[EXIT]], label [[LOOP_SPLIT]]
+; CHECK:       loop.split:
+; CHECK-NEXT:    br i1 [[C1]], label [[LOOP_LATCH:%.*]], label [[LOOP:%.*]]
+; CHECK:       loop.latch:
+; CHECK-NEXT:    call void @foo()
+; CHECK-NEXT:    br label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br i1 %c1, label %exit, label %if
+
+if:
+  call void @foo()
+  br i1 %c2, label %exit, label %loop
+
+loop:
+  %c3 = icmp eq i64 0, 0
+  br i1 %c3, label %exit, label %loop.split
+
+loop.split:
+  br i1 %c1, label %loop.latch, label %loop
+
+loop.latch:
+  call void @foo()
+  br label %loop
+
+exit:
+  ret void
+}

diff  --git a/llvm/test/Transforms/SimplifyCFG/pr55765.ll b/llvm/test/Transforms/SimplifyCFG/pr55765.ll
index 36d7ed1dc536c..ffe1ed9915bc8 100644
--- a/llvm/test/Transforms/SimplifyCFG/pr55765.ll
+++ b/llvm/test/Transforms/SimplifyCFG/pr55765.ll
@@ -8,15 +8,24 @@ declare void @dummy()
 
 define i32 @main(i1 %c1, i1 %c2, i32 %y) {
 ; CHECK-LABEL: @main(
-; CHECK-NEXT:    [[C1_NOT:%.*]] = xor i1 [[C1:%.*]], true
+; CHECK-NEXT:    br i1 [[C1:%.*]], label [[EXIT:%.*]], label [[LOOP_PRE_PREHEADER:%.*]]
+; CHECK:       loop.pre.preheader:
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i32 [[Y:%.*]], -1
-; CHECK-NEXT:    [[OR_COND:%.*]] = select i1 [[C1_NOT]], i1 [[CMP]], i1 false
-; CHECK-NEXT:    br i1 [[OR_COND]], label [[LOOP_PREHEADER:%.*]], label [[EXIT:%.*]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[LOOP_PREHEADER:%.*]], label [[EXIT]]
 ; CHECK:       loop.preheader:
 ; CHECK-NEXT:    [[CMP2:%.*]] = icmp eq i32 [[Y]], 0
-; CHECK-NEXT:    br label [[LOOP_CRITEDGE:%.*]]
-; CHECK:       loop.critedge:
-; CHECK-NEXT:    br label [[LOOP_CRITEDGE]]
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    br i1 [[C1]], label [[LOOP2:%.*]], label [[LOOP_LATCH:%.*]]
+; CHECK:       loop.latch:
+; CHECK-NEXT:    br i1 [[CMP]], label [[LOOP]], label [[EXIT]]
+; CHECK:       loop2:
+; CHECK-NEXT:    br i1 [[CMP2]], label [[JOIN:%.*]], label [[IF:%.*]]
+; CHECK:       if:
+; CHECK-NEXT:    call void @dummy()
+; CHECK-NEXT:    br label [[JOIN]]
+; CHECK:       join:
+; CHECK-NEXT:    br i1 [[C2:%.*]], label [[LOOP2]], label [[LOOP_LATCH]]
 ; CHECK:       exit:
 ; CHECK-NEXT:    ret i32 0
 ;

diff  --git a/llvm/test/Transforms/SimplifyCFG/wc-widen-block.ll b/llvm/test/Transforms/SimplifyCFG/wc-widen-block.ll
index fe33141502a6b..ea4abedaf88fd 100644
--- a/llvm/test/Transforms/SimplifyCFG/wc-widen-block.ll
+++ b/llvm/test/Transforms/SimplifyCFG/wc-widen-block.ll
@@ -269,18 +269,17 @@ return:
 define i32 @neg_loop(i1 %cond_0, i1 %cond_1) {
 ; CHECK-LABEL: @neg_loop(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    call void @unknown()
-; CHECK-NEXT:    br i1 [[COND_1:%.*]], label [[LOOP:%.*]], label [[DEOPT2:%.*]], !prof [[PROF0]]
-; CHECK:       loop.critedge:
-; CHECK-NEXT:    call void @unknown()
-; CHECK-NEXT:    br label [[LOOP]]
+; CHECK-NEXT:    br label [[GUARDED:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    [[WIDENABLE_COND:%.*]] = call i1 @llvm.experimental.widenable.condition()
 ; CHECK-NEXT:    [[EXIPLICIT_GUARD_COND:%.*]] = and i1 [[COND_0:%.*]], [[WIDENABLE_COND]]
-; CHECK-NEXT:    br i1 [[EXIPLICIT_GUARD_COND]], label [[LOOP_CRITEDGE:%.*]], label [[DEOPT:%.*]], !prof [[PROF0]]
+; CHECK-NEXT:    br i1 [[EXIPLICIT_GUARD_COND]], label [[GUARDED]], label [[DEOPT:%.*]], !prof [[PROF0]]
 ; CHECK:       deopt:
 ; CHECK-NEXT:    [[DEOPTRET:%.*]] = call i32 (...) @llvm.experimental.deoptimize.i32() [ "deopt"() ]
 ; CHECK-NEXT:    ret i32 [[DEOPTRET]]
+; CHECK:       guarded:
+; CHECK-NEXT:    call void @unknown()
+; CHECK-NEXT:    br i1 [[COND_1:%.*]], label [[LOOP:%.*]], label [[DEOPT2:%.*]], !prof [[PROF0]]
 ; CHECK:       deopt2:
 ; CHECK-NEXT:    [[DEOPTRET2:%.*]] = call i32 (...) @llvm.experimental.deoptimize.i32() [ "deopt"() ]
 ; CHECK-NEXT:    ret i32 [[DEOPTRET2]]


        


More information about the llvm-commits mailing list