diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp --- a/llvm/lib/CodeGen/MachineVerifier.cpp +++ b/llvm/lib/CodeGen/MachineVerifier.cpp @@ -569,16 +569,6 @@ verifyStackFrame(); } -// Does iterator point to a and b as the first two elements? -static bool matchPair(MachineBasicBlock::const_succ_iterator i, - const MachineBasicBlock *a, const MachineBasicBlock *b) { - if (*i == a) - return *++i == b; - if (*i == b) - return *++i == a; - return false; -} - void MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) { FirstTerminator = nullptr; @@ -612,20 +602,6 @@ } } - // Count the number of INLINEASM_BR indirect target successors. - SmallPtrSet IndirectTargetSuccs; - for (const auto *succ : MBB->successors()) { - if (MBB->isInlineAsmBrIndirectTarget(succ)) - IndirectTargetSuccs.insert(succ); - if (!FunctionBlocks.count(succ)) - report("MBB has successor that isn't part of the function.", MBB); - if (!MBBInfoMap[succ].Preds.count(MBB)) { - report("Inconsistent CFG", MBB); - errs() << "MBB is not in the predecessor list of the successor " - << printMBBReference(*succ) << ".\n"; - } - } - // Check the predecessor list. for (const MachineBasicBlock *Pred : MBB->predecessors()) { if (!FunctionBlocks.count(Pred)) @@ -656,26 +632,6 @@ // check whether its answers match up with reality. if (!TBB && !FBB) { // Block falls through to its successor. - MachineFunction::const_iterator MBBI = std::next(MBB->getIterator()); - if (MBBI == MF->end()) { - // It's possible that the block legitimately ends with a noreturn - // call or an unreachable, in which case it won't actually fall - // out the bottom of the function. - } else if (MBB->succ_size() == LandingPadSuccs.size() || - MBB->succ_size() == IndirectTargetSuccs.size()) { - // It's possible that the block legitimately ends with a noreturn - // call or an unreachable, in which case it won't actually fall - // out of the block. - } else if ((LandingPadSuccs.size() && - MBB->succ_size() != 1 + LandingPadSuccs.size()) || - (IndirectTargetSuccs.size() && - MBB->succ_size() != 1 + IndirectTargetSuccs.size())) { - report("MBB exits via unconditional fall-through but doesn't have " - "exactly one CFG successor!", MBB); - } else if (!MBB->isSuccessor(&*MBBI)) { - report("MBB exits via unconditional fall-through but its successor " - "differs from its CFG successor!", MBB); - } if (!MBB->empty() && MBB->back().isBarrier() && !TII->isPredicated(MBB->back())) { report("MBB exits via unconditional fall-through but ends with a " @@ -687,20 +643,6 @@ } } else if (TBB && !FBB && Cond.empty()) { // Block unconditionally branches somewhere. - // 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()) && - MBB->succ_size() != 1 + IndirectTargetSuccs.size() && - (MBB->succ_size() != 1 || IndirectTargetSuccs.size() != 1 || - *MBB->succ_begin() != *IndirectTargetSuccs.begin())) { - report("MBB exits via unconditional branch but doesn't have " - "exactly one CFG successor!", MBB); - } else if (!MBB->isSuccessor(TBB)) { - report("MBB exits via unconditional branch but the CFG " - "successor doesn't match the actual successor!", MBB); - } if (MBB->empty()) { report("MBB exits via unconditional branch but doesn't contain " "any instructions!", MBB); @@ -713,24 +655,6 @@ } } else if (TBB && !FBB && !Cond.empty()) { // Block conditionally branches somewhere, otherwise falls through. - MachineFunction::const_iterator MBBI = std::next(MBB->getIterator()); - if (MBBI == MF->end()) { - report("MBB conditionally falls through out of function!", MBB); - } else if (MBB->succ_size() == 1) { - // A conditional branch with only one successor is weird, but allowed. - if (&*MBBI != TBB) - report("MBB exits via conditional branch/fall-through but only has " - "one CFG successor!", MBB); - else if (TBB != *MBB->succ_begin()) - report("MBB exits via conditional branch/fall-through but the CFG " - "successor don't match the actual successor!", MBB); - } else if (MBB->succ_size() != 2) { - report("MBB exits via conditional branch/fall-through but doesn't have " - "exactly two CFG successors!", MBB); - } else if (!matchPair(MBB->succ_begin(), TBB, &*MBBI)) { - report("MBB exits via conditional branch/fall-through but the CFG " - "successors don't match the actual successors!", MBB); - } if (MBB->empty()) { report("MBB exits via conditional branch/fall-through but doesn't " "contain any instructions!", MBB); @@ -744,21 +668,6 @@ } else if (TBB && FBB) { // Block conditionally branches somewhere, otherwise branches // somewhere else. - if (MBB->succ_size() == 1) { - // A conditional branch with only one successor is weird, but allowed. - if (FBB != TBB) - report("MBB exits via conditional branch/branch through but only has " - "one CFG successor!", MBB); - else if (TBB != *MBB->succ_begin()) - report("MBB exits via conditional branch/branch through but the CFG " - "successor don't match the actual successor!", MBB); - } else if (MBB->succ_size() != 2) { - report("MBB exits via conditional branch/branch but doesn't have " - "exactly two CFG successors!", MBB); - } else if (!matchPair(MBB->succ_begin(), TBB, FBB)) { - report("MBB exits via conditional branch/branch but the CFG " - "successors don't match the actual successors!", MBB); - } if (MBB->empty()) { report("MBB exits via conditional branch/branch but doesn't " "contain any instructions!", MBB); @@ -776,6 +685,53 @@ } else { report("analyzeBranch returned invalid data!", MBB); } + + // Now check that the successors match up with the answers reported by + // analyzeBranch. + if (TBB && !MBB->isSuccessor(TBB)) + report("MBB exits via jump or conditional branch, but its target isn't a " + "CFG successor!", + MBB); + if (FBB && !MBB->isSuccessor(FBB)) + report("MBB exits via conditional branch, but its target isn't a CFG " + "successor!", + MBB); + + // There might be a fallthrough to the next block if there's either no + // unconditional true branch, or if there's a condition, and one of the + // branches is missing. + bool Fallthrough = !TBB || (!Cond.empty() && !FBB); + + // A conditional fallthrough must be an actual CFG successor, not + // unreachable. (Conversely, an unconditional fallthrough might not really + // be a successor, because the block might end in unreachable.) + if (!Cond.empty() && !FBB) { + MachineFunction::const_iterator MBBI = std::next(MBB->getIterator()); + if (MBBI == MF->end()) { + report("MBB conditionally falls through out of function!", MBB); + } else if (!MBB->isSuccessor(&*MBBI)) + report("MBB exits via conditional branch/fall-through but the CFG " + "successors don't match the actual successors!", + MBB); + } + + // Verify that there aren't any extra un-accounted-for successors. + for (const MachineBasicBlock *SuccMBB : MBB->successors()) { + // If this successor is one of the branch targets, it's okay. + if (SuccMBB == TBB || SuccMBB == FBB) + continue; + // If we might have a fallthrough, and the successor is the fallthrough + // block, that's also ok. + if (Fallthrough && SuccMBB == MBB->getNextNode()) + continue; + // Also accept successors which are for exception-handling or might be + // inlineasm_br targets. + if (SuccMBB->isEHPad() || SuccMBB->isInlineAsmBrIndirectTarget()) + continue; + report("MBB has unexpected successors which are not branch targets, " + "fallthrough, EHPads, or inlineasm_br targets.", + MBB); + } } regsLive.clear(); diff --git a/llvm/test/CodeGen/Hexagon/cext-opt-range-offset.mir b/llvm/test/CodeGen/Hexagon/cext-opt-range-offset.mir --- a/llvm/test/CodeGen/Hexagon/cext-opt-range-offset.mir +++ b/llvm/test/CodeGen/Hexagon/cext-opt-range-offset.mir @@ -36,7 +36,6 @@ successors: %bb.4 bb.4: - successors: %bb.4 %5 = A2_tfrsi -234944521 %6 = A2_tfrsi -360185632 L4_and_memopw_io %6, 0, %5 diff --git a/llvm/test/CodeGen/WebAssembly/eh-labels.mir b/llvm/test/CodeGen/WebAssembly/eh-labels.mir --- a/llvm/test/CodeGen/WebAssembly/eh-labels.mir +++ b/llvm/test/CodeGen/WebAssembly/eh-labels.mir @@ -29,6 +29,7 @@ EH_LABEL CALL @foo, implicit-def dead $arguments, implicit $sp32, implicit $sp64 EH_LABEL + BR %bb.2, implicit-def dead $arguments bb.1 (landing-pad): ; predecessors: %bb.0 diff --git a/llvm/test/MachineVerifier/verifier-pseudo-terminators.mir b/llvm/test/MachineVerifier/verifier-pseudo-terminators.mir --- a/llvm/test/MachineVerifier/verifier-pseudo-terminators.mir +++ b/llvm/test/MachineVerifier/verifier-pseudo-terminators.mir @@ -4,7 +4,8 @@ # Make sure that mismatched successors are caught when a _term # instruction is used -# CHECK: *** Bad machine code: MBB exits via unconditional branch but the CFG successor doesn't match the actual successor! *** +# CHECK: *** Bad machine code: MBB exits via jump or conditional branch, but its target isn't a CFG successor! *** +# CHECK: *** Bad machine code: MBB has unexpected successors which are not branch targets, fallthrough, EHPads, or inlineasm_br targets. *** --- name: verifier_pseudo_terminators