Skip to content

Commit

Permalink
Revert "[SimplifyCFG] Thread branches on same condition in more cases…
Browse files Browse the repository at this point in the history
… (PR54980)"

This reverts commit 4e545bd.

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 #56203.
  • Loading branch information
nikic committed Jul 5, 2022
1 parent 6c3c5f8 commit a4772cb
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 61 deletions.
15 changes: 7 additions & 8 deletions clang/test/CodeGenObjC/exceptions.m
Expand Up @@ -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;
Expand All @@ -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]]
Expand All @@ -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--;
}

Expand Down
11 changes: 6 additions & 5 deletions clang/test/CodeGenObjCXX/exceptions-legacy.mm
Expand Up @@ -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

27 changes: 4 additions & 23 deletions llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/AArch64/arm64-andCmpBrToTBZ.ll
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Transforms/GVNSink/sink-common-code.ll
Expand Up @@ -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) {

Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
Expand Up @@ -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:
Expand Down
80 changes: 71 additions & 9 deletions llvm/test/Transforms/SimplifyCFG/jump-threading.ll
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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]]
Expand Down Expand Up @@ -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]]
Expand Down Expand Up @@ -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
}
21 changes: 15 additions & 6 deletions llvm/test/Transforms/SimplifyCFG/pr55765.ll
Expand Up @@ -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
;
Expand Down
11 changes: 5 additions & 6 deletions llvm/test/Transforms/SimplifyCFG/wc-widen-block.ll
Expand Up @@ -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]]
Expand Down

0 comments on commit a4772cb

Please sign in to comment.