From 3b7b209bf86d3e81d61cc195020bd4891467291b Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Tue, 22 Nov 2011 13:13:16 +0000 Subject: Fix a devilish miscompile exposed by block placement. The updateTerminator code didn't correctly handle EH terminators in one very specific case. AnalyzeBranch would find no terminator instruction, and so the fallback in updateTerminator is to assume fallthrough. This is correct, but the destination of the fallthrough was assumed to be the first successor. This is *almost always* true, but in certain cases the loop transformations will cause the landing pad to be the first successor! Instead of this brittle logic, actually look through the successors for a non-landing-pad accessor, and to assert if more than one is found. This will hopefully fix some (if not all) of the self host miscompiles with block placement. Thanks to Benjamin Kramer for reporting, Nick Lewycky for an initial stab at a reduction, and Duncan for endless advice on EH (which I know nothing about) as well as reviewing the actual fix. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@145062 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/MachineBasicBlock.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'lib/CodeGen/MachineBasicBlock.cpp') diff --git a/lib/CodeGen/MachineBasicBlock.cpp b/lib/CodeGen/MachineBasicBlock.cpp index 4c5fe4c..f05b6c1 100644 --- a/lib/CodeGen/MachineBasicBlock.cpp +++ b/lib/CodeGen/MachineBasicBlock.cpp @@ -297,8 +297,14 @@ void MachineBasicBlock::updateTerminator() { TII->RemoveBranch(*this); } else { // The block has an unconditional fallthrough. If its successor is not - // its layout successor, insert a branch. - TBB = *succ_begin(); + // its layout successor, insert a branch. First we have to locate the + // only non-landing-pad successor, as that is the fallthrough block. + for (succ_iterator SI = succ_begin(), SE = succ_end(); SI != SE; ++SI) { + if ((*SI)->isLandingPad()) + continue; + assert(!TBB && "Found more than one non-landing-pad successor!"); + TBB = *SI; + } if (!isLayoutSuccessor(TBB)) TII->InsertBranch(*this, TBB, 0, Cond, dl); } -- cgit v1.1 From 521fc5bcd73489f604a0b3251247c5ef21b5a0a5 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Wed, 23 Nov 2011 08:23:54 +0000 Subject: Handle the case of a no-return invoke correctly. It actually still has successors, they just are all landing pad successors. We handle this the same way as no successors. Comments attached for the next person to wade through here and another lovely test case courtesy of Benjamin Kramer's bugpoint reduction. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@145098 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/MachineBasicBlock.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'lib/CodeGen/MachineBasicBlock.cpp') diff --git a/lib/CodeGen/MachineBasicBlock.cpp b/lib/CodeGen/MachineBasicBlock.cpp index f05b6c1..b9d1ef7 100644 --- a/lib/CodeGen/MachineBasicBlock.cpp +++ b/lib/CodeGen/MachineBasicBlock.cpp @@ -305,6 +305,14 @@ void MachineBasicBlock::updateTerminator() { assert(!TBB && "Found more than one non-landing-pad successor!"); TBB = *SI; } + + // If there is no non-landing-pad successor, the block has no + // fall-through edges to be concerned with. + if (!TBB) + 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, 0, Cond, dl); } -- cgit v1.1