From a4772cbaf0dc717ab6b4639272ca2910897613f0 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 5 Jul 2022 10:56:54 +0200 Subject: [PATCH] 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. --- clang/test/CodeGenObjC/exceptions.m | 15 ++-- clang/test/CodeGenObjCXX/exceptions-legacy.mm | 11 +-- llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 27 +------ .../CodeGen/AArch64/arm64-andCmpBrToTBZ.ll | 2 +- .../Transforms/GVNSink/sink-common-code.ll | 2 +- .../SimplifyCFG/X86/sink-common-code.ll | 4 +- .../Transforms/SimplifyCFG/jump-threading.ll | 80 ++++++++++++++++--- llvm/test/Transforms/SimplifyCFG/pr55765.ll | 21 +++-- .../Transforms/SimplifyCFG/wc-widen-block.ll | 11 ++- 9 files changed, 112 insertions(+), 61 deletions(-) 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, - 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(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(U)) KnownValues[CB].insert(PN->getIncomingBlock(U)); } else { - SmallDenseMap, 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_different_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]]