[LLVMdev] Should the MachineVerifier accept a MBB with a single (landing pad) successor?

[Ahmed Bougacha]

Hi all,

I've been investigating a machine verifier failure on the attached
testcase, and I'm tempted to say the verifier is wrong and should
accept it.  Skip the description for the proposed change.


On AArch64, the verifier complains with:

    *** Bad machine code: MBB exits via unconditional branch but
doesn't have exactly one CFG successor! ***
    - function:    t4
    - basic block: BB#5 invoke.cont41

The freshly selected relevant blocks are:

    BB#7: derived from LLVM BB %invoke.cont41
            EH_LABEL <MCSym=Ltmp4>
            B <BB#8>
        Successors according to CFG: BB#8(1) BB#9(1)

    BB#8: derived from LLVM BB %invoke.cont43
        Predecessors according to CFG: BB#7

    BB#9: derived from LLVM BB %lpad40, EH LANDING PAD
        Predecessors according to CFG: BB#7
            EH_LABEL <MCSym=Ltmp5>
            ...


The unreachable BB#8 gets removed, and we end up with:

    BB#5: derived from LLVM BB %invoke.cont41
            ...
            B <BB#8>
        Successors according to CFG: BB#8(2)
    ...
    BB#8: derived from LLVM BB %lpad40, EH LANDING PAD
        Predecessors according to CFG: BB#5


On other targets, the branch terminating BB#7 gets removed early, for
various reasons (happenstance, really).  The edge to the landing pad
is still there, but the verifier can't check anything because there's
no branch to analyze.

On AArch64, the branch remains, and the verifier hits this condition:

      if (MBB->succ_size() != 1+LandingPadSuccs.size())

So:
- the problem exists elsewhere, but is hidden by branch folder optimizations
- if the normal successor to an invoke BB is unreachable, it seems
reasonable to only have 1 successor, the landing pad.

Hence my simple change, making the verifier accept it:

--- c/lib/CodeGen/MachineVerifier.cpp
+++ w/lib/CodeGen/MachineVerifier.cpp
@@ -590,7 +590,11 @@
MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock
*MBB) {
       }
     } else if (TBB && !FBB && Cond.empty()) {
       // Block unconditionally branches somewhere.
-      if (MBB->succ_size() != 1+LandingPadSuccs.size()) {
+      // If the block has exactly one successor, that happens to be a
+      // landingpad, accept it as valid control flow.
+      if (MBB->succ_size() != 1+LandingPadSuccs.size() &&
+          (MBB->succ_size() != 1 || LandingPadSuccs.size() != 1 ||
+           *MBB->succ_begin() != *LandingPadSuccs.begin())) {
         report("MBB exits via unconditional branch but doesn't have "
                "exactly one CFG successor!", MBB);
       } else if (!MBB->isSuccessor(TBB)) {


- Ahmed
-------------- next part --------------
A non-text attachment was scrubbed...
Name: br-simple.ll
Type: application/octet-stream
Size: 6466 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141106/a7d178a6/attachment.obj>