RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2025-01-31 12:41:49 +01:00
Code Issues Projects Releases Wiki Activity

Simplify MachineVerifier's block-successor verification.

Browse Source 
There's two properties we want to verify:

1. That the successors returned by analyzeBranch are in the CFG
   successor list, and
2. That there are no extraneous successors are in the CFG successor
   list.

The previous implementation mostly accomplished this, but in a very
convoluted manner.

Differential Revision: https://reviews.llvm.org/D79793
This commit is contained in:
James Y Knight 2020-05-12 10:36:34 -04:00
parent f92fad214d
commit ae071ee9e2
4 changed files with 50 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

138
lib/CodeGen/MachineVerifier.cpp
View File

@ -573,16 +573,6 @@ void MachineVerifier::visitMachineFunctionBefore() {
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;
@ -616,20 +606,6 @@ MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) {
}
}
// Count the number of INLINEASM_BR indirect target successors.
SmallPtrSet<const MachineBasicBlock*, 4> 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))
@ -660,26 +636,6 @@ MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) {
// 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 "
@ -691,20 +647,6 @@ MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) {
}
} 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);
@ -717,24 +659,6 @@ MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) {
}
} 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);
@ -748,21 +672,6 @@ MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) {
} 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);
@ -780,6 +689,53 @@ MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) {
} 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() || MBB->isInlineAsmBrIndirectTarget(SuccMBB))
continue;
report("MBB has unexpected successors which are not branch targets, "
"fallthrough, EHPads, or inlineasm_br targets.",
MBB);
}
}
regsLive.clear();

1
test/CodeGen/Hexagon/cext-opt-range-offset.mir
View File

@ -36,7 +36,6 @@ body: |
successors: %bb.4
bb.4:
successors: %bb.4
%5 = A2_tfrsi -234944521
%6 = A2_tfrsi -360185632
L4_and_memopw_io %6, 0, %5

1
test/CodeGen/WebAssembly/eh-labels.mir
View File

@ -29,6 +29,7 @@ body: |
EH_LABEL <mcsymbol .Ltmp0>
CALL @foo, implicit-def dead $arguments, implicit $sp32, implicit $sp64
EH_LABEL <mcsymbol .Ltmp1>
BR %bb.2, implicit-def dead $arguments
bb.1 (landing-pad):
; predecessors: %bb.0

3
test/MachineVerifier/verifier-pseudo-terminators.mir
View File

@ -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