From b71b2f622a00ab55073214e4e701f902ebeb8dc9 Mon Sep 17 00:00:00 2001 From: Haicheng Wu Date: Sun, 3 Jul 2016 19:14:17 +0000 Subject: [PATCH] [MBB] add a missing corner case in UpdateTerminator() After the block placement, if a block ends with a conditional branch, but the next block is not its successor. The conditional branch should be changed to unconditional branch. This patch fixes PR28307, PR28297, PR28402. Differential Revision: http://reviews.llvm.org/D21811 llvm-svn: 274470 --- llvm/lib/CodeGen/MachineBasicBlock.cpp | 30 +++++++---- llvm/test/CodeGen/X86/update-terminator.mir | 57 +++++++++++++++++++++ 2 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 llvm/test/CodeGen/X86/update-terminator.mir diff --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp index e2d0c4b7e85e..d4453c71da29 100644 --- a/llvm/lib/CodeGen/MachineBasicBlock.cpp +++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp @@ -470,17 +470,27 @@ void MachineBasicBlock::updateTerminator() { FallthroughBB = *SI; } - if (!FallthroughBB && canFallThrough()) { - // We fallthrough to the same basic block as the conditional jump targets. - // Remove the conditional jump, leaving unconditional fallthrough. - // FIXME: This does not seem like a reasonable pattern to support, but it - // has been seen in the wild coming out of degenerate ARM test cases. - TII->RemoveBranch(*this); + if (!FallthroughBB) { + if (canFallThrough()) { + // We fallthrough to the same basic block as the conditional jump targets. + // Remove the conditional jump, leaving unconditional fallthrough. + // FIXME: This does not seem like a reasonable pattern to support, but it + // has been seen in the wild coming out of degenerate ARM test cases. + TII->RemoveBranch(*this); + + // Finally update the unconditional successor to be reached via a branch if + // it would not be reached by fallthrough. + if (!isLayoutSuccessor(TBB)) + TII->InsertBranch(*this, TBB, nullptr, Cond, DL); + return; + } - // Finally update the unconditional successor to be reached via a branch if - // it would not be reached by fallthrough. - if (!isLayoutSuccessor(TBB)) - TII->InsertBranch(*this, TBB, nullptr, Cond, DL); + // We enter here iff exactly one successor is TBB which cannot fallthrough + // and the rest successors if any are EHPads. In this case, we need to + // change the conditional branch into unconditional branch. + TII->RemoveBranch(*this); + Cond.clear(); + TII->InsertBranch(*this, TBB, nullptr, Cond, DL); return; } diff --git a/llvm/test/CodeGen/X86/update-terminator.mir b/llvm/test/CodeGen/X86/update-terminator.mir new file mode 100644 index 000000000000..72898cf74b4b --- /dev/null +++ b/llvm/test/CodeGen/X86/update-terminator.mir @@ -0,0 +1,57 @@ +# RUN: llc -march=x86-64 -verify-machineinstrs -run-pass block-placement -o /dev/null %s 2>&1 | FileCheck %s +# Check the conditional jump in bb.1 is changed to unconditional after block placement swaps bb.2 and bb.3. + +--- | + @a = external global i16 + @b = external global i32 + + ; Function Attrs: nounwind + define void @f2() { + br i1 undef, label %bb1, label %bb3 + + bb1: + br i1 undef, label %bb2, label %bb2 + + bb2: + br label %bb4 + + bb3: + br label %bb2 + + bb4: + ret void + } + + +... +--- +# CHECK-LABEL: name: f2 +# CHECK: bb.1: +# CHECK: JMP_1 %bb.2 +# CHECK: bb.3: +# CHECK: bb.2: +name: f2 +body: | + bb.0 (%ir-block.0): + successors: %bb.1(50), %bb.3(50) + + JNE_1 %bb.1, implicit %eflags + JMP_1 %bb.3 + bb.1: + successors: %bb.2(100) + + JNE_1 %bb.2, implicit %eflags + + bb.2: + successors: %bb.4(100) + + JMP_1 %bb.4 + + bb.3: + successors: %bb.2(100) + JMP_1 %bb.2 + + bb.4: + RETQ + +...