Page MenuHomePhabricator
Differential D75098 Diff 261771 llvm/lib/CodeGen/MachineVerifier.cpp

Changeset View

Standalone View

View Options

llvm/lib/CodeGen/MachineVerifier.cpp

Show First 20 Lines • Show All 607 Lines • ▼ Show 20 Lines for (const auto *succ : MBB->successors()) {
if (!MBBInfoMap[succ].Preds.count(MBB)) { if (!MBBInfoMap[succ].Preds.count(MBB)) {
report("Inconsistent CFG", MBB); report("Inconsistent CFG", MBB);
errs() << "MBB is not in the predecessor list of the successor " errs() << "MBB is not in the predecessor list of the successor "
<< printMBBReference(*succ) << ".\n"; << printMBBReference(*succ) << ".\n";
} }
} }
// Count the number of INLINEASM_BR indirect target successors. // Count the number of INLINEASM_BR indirect target successors.
SmallPtrSet<const MachineBasicBlock*, 4> IndirectTargetSuccs; unsigned NumIndirectTargetSuccs = 0;
const MachineBasicBlock *IAB = MBB;
if (IAB->pred_size() == 1 &&
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-  if (IAB->pred_size() == 1 &&
-      llvm::any_of((*IAB->pred_begin())->terminators(),
-                   [](const MachineInstr &MI){
-                     return MI.getOpcode() == TargetOpcode::INLINEASM_BR;
-                   }))
+  if (IAB->pred_size() == 1 && llvm::any_of((*IAB->pred_begin())->terminators(),
+                                            [](const MachineInstr &MI) {
+                                              return MI.getOpcode() ==
+                                                     TargetOpcode::INLINEASM_BR;
+                                            }))
Lint: Pre-merge checks: clang-format: please reformat the code ``` - if (IAB->pred_size() == 1 && - llvm::any_of…
llvm::any_of((*IAB->pred_begin())->terminators(),
[](const MachineInstr &MI){
return MI.getOpcode() == TargetOpcode::INLINEASM_BR;
}))
IAB = *IAB->pred_begin();
for (const auto &term : IAB->terminators()) {
if (term.getOpcode() != TargetOpcode::INLINEASM_BR)
continue;
if (MBB->succ_size() != 1)
for (const auto &mo : term.operands())
if (mo.isBlockAddress())
++NumIndirectTargetSuccs;
break;
}
arsenmUnsubmitted
Not Done
ReplyInline Actions

This also looks like a separate change

arsenm: This also looks like a separate change
for (const auto *succ : MBB->successors()) { for (const auto *succ : MBB->successors()) {
if (MBB->isInlineAsmBrIndirectTarget(succ))
IndirectTargetSuccs.insert(succ);
if (!FunctionBlocks.count(succ)) if (!FunctionBlocks.count(succ))
report("MBB has successor that isn't part of the function.", MBB); report("MBB has successor that isn't part of the function.", MBB);
if (!MBBInfoMap[succ].Preds.count(MBB)) { if (!MBBInfoMap[succ].Preds.count(MBB)) {
report("Inconsistent CFG", MBB); report("Inconsistent CFG", MBB);
errs() << "MBB is not in the predecessor list of the successor " errs() << "MBB is not in the predecessor list of the successor "
<< printMBBReference(*succ) << ".\n"; << printMBBReference(*succ) << ".\n";
} }
} }
Show All 29 Lines if (!TII->analyzeBranch(*const_cast<MachineBasicBlock *>(MBB), TBB, FBB,
if (!TBB && !FBB) { if (!TBB && !FBB) {
// Block falls through to its successor. // Block falls through to its successor.
MachineFunction::const_iterator MBBI = std::next(MBB->getIterator()); MachineFunction::const_iterator MBBI = std::next(MBB->getIterator());
if (MBBI == MF->end()) { if (MBBI == MF->end()) {
// It's possible that the block legitimately ends with a noreturn // It's possible that the block legitimately ends with a noreturn
// call or an unreachable, in which case it won't actually fall // call or an unreachable, in which case it won't actually fall
// out the bottom of the function. // out the bottom of the function.
} else if (MBB->succ_size() == LandingPadSuccs.size() || } else if (MBB->succ_size() == LandingPadSuccs.size() ||
MBB->succ_size() == IndirectTargetSuccs.size()) { MBB->succ_size() == NumIndirectTargetSuccs) {
// It's possible that the block legitimately ends with a noreturn // It's possible that the block legitimately ends with a noreturn
// call or an unreachable, in which case it won't actually fall // call or an unreachable, in which case it won't actually fall
// out of the block. // out of the block.
} else if ((LandingPadSuccs.size() && } else if ((LandingPadSuccs.size() &&
MBB->succ_size() != 1 + LandingPadSuccs.size()) || MBB->succ_size() != 1 + LandingPadSuccs.size()) ||
(IndirectTargetSuccs.size() && (NumIndirectTargetSuccs &&
MBB->succ_size() != 1 + IndirectTargetSuccs.size())) { MBB->succ_size() != 1 + NumIndirectTargetSuccs)) {
report("MBB exits via unconditional fall-through but doesn't have " report("MBB exits via unconditional fall-through but doesn't have "
"exactly one CFG successor!", MBB); "exactly one CFG successor!", MBB);
} else if (!MBB->isSuccessor(&*MBBI)) { } else if (!MBB->isSuccessor(&*MBBI)) {
report("MBB exits via unconditional fall-through but its successor " report("MBB exits via unconditional fall-through but its successor "
"differs from its CFG successor!", MBB); "differs from its CFG successor!", MBB);
} }
if (!MBB->empty() && MBB->back().isBarrier() && if (!MBB->empty() && MBB->back().isBarrier() &&
!TII->isPredicated(MBB->back())) { !TII->isPredicated(MBB->back())) {
report("MBB exits via unconditional fall-through but ends with a " report("MBB exits via unconditional fall-through but ends with a "
"barrier instruction!", MBB); "barrier instruction!", MBB);
} }
if (!Cond.empty()) { if (!Cond.empty()) {
report("MBB exits via unconditional fall-through but has a condition!", report("MBB exits via unconditional fall-through but has a condition!",
MBB); MBB);
} }
} else if (TBB && !FBB && Cond.empty()) { } else if (TBB && !FBB && Cond.empty()) {
// Block unconditionally branches somewhere. // Block unconditionally branches somewhere.
// If the block has exactly one successor, that happens to be a // If the block has exactly one successor, that happens to be a
// landingpad, accept it as valid control flow. // landingpad, accept it as valid control flow.
if (MBB->succ_size() != 1+LandingPadSuccs.size() && if (MBB->succ_size() != 1+LandingPadSuccs.size() &&
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-      if (MBB->succ_size() != 1+LandingPadSuccs.size() &&
+      if (MBB->succ_size() != 1 + LandingPadSuccs.size() &&
Lint: Pre-merge checks: clang-format: please reformat the code ``` - if (MBB->succ_size() != 1+LandingPadSuccs.
(MBB->succ_size() != 1 || LandingPadSuccs.size() != 1 || (MBB->succ_size() != 1 || LandingPadSuccs.size() != 1 ||
*MBB->succ_begin() != *LandingPadSuccs.begin()) && *MBB->succ_begin() != *LandingPadSuccs.begin()) &&
MBB->succ_size() != 1 + IndirectTargetSuccs.size() && MBB->succ_size() != 1 + NumIndirectTargetSuccs &&
(MBB->succ_size() != 1 || IndirectTargetSuccs.size() != 1 || (MBB->succ_size() != 1 || NumIndirectTargetSuccs != 1)) {
*MBB->succ_begin() != *IndirectTargetSuccs.begin())) {
report("MBB exits via unconditional branch but doesn't have " report("MBB exits via unconditional branch but doesn't have "
"exactly one CFG successor!", MBB); "exactly one CFG successor!", MBB);
} else if (!MBB->isSuccessor(TBB)) { } else if (!MBB->isSuccessor(TBB)) {
report("MBB exits via unconditional branch but the CFG " report("MBB exits via unconditional branch but the CFG "
"successor doesn't match the actual successor!", MBB); "successor doesn't match the actual successor!", MBB);
} }
if (MBB->empty()) { if (MBB->empty()) {
report("MBB exits via unconditional branch but doesn't contain " report("MBB exits via unconditional branch but doesn't contain "
▲ Show 20 Lines • Show All 105 Lines • ▼ Show 20 Lines if (Indexes && Indexes->hasIndex(*MI)) {
SlotIndex idx = Indexes->getInstructionIndex(*MI); SlotIndex idx = Indexes->getInstructionIndex(*MI);
if (!(idx > lastIndex)) { if (!(idx > lastIndex)) {
report("Instruction index out of order", MI); report("Instruction index out of order", MI);
errs() << "Last instruction was at " << lastIndex << '\n'; errs() << "Last instruction was at " << lastIndex << '\n';
} }
lastIndex = idx; lastIndex = idx;
} }
// Ensure non-terminators don't follow terminators. // Ensure non-terminators don't follow terminators. Ignore predicated
// Ignore predicated terminators formed by if conversion. // terminators formed by if conversion.
//
// FIXME: If conversion shouldn't need to violate this rule. // FIXME: If conversion shouldn't need to violate this rule.
if (MI->isTerminator() && !TII->isPredicated(*MI)) { if (MI->isTerminator() && !TII->isPredicated(*MI)) {
if (!FirstTerminator) if (!FirstTerminator)
FirstTerminator = MI; FirstTerminator = MI;
} else if (FirstTerminator) { } else if (FirstTerminator) {
// Ignore stack dumps after a terminator at -O0. These are most likely from
// a TCOPY.
if (TM->getOptLevel() == CodeGenOpt::None && MI->mayStore()) {
FirstTerminator = MI;
nickdesaulniersUnsubmitted
Done
ReplyInline Actions

this is hard to read. Is there a nice way to simply this? Maybe negate, and fold into parent if?

nickdesaulniers: this is hard to read. Is there a nice way to simply this? Maybe negate, and fold into parent…
} else {
report("Non-terminator instruction after the first terminator", MI); report("Non-terminator instruction after the first terminator", MI);
errs() << "First terminator was:\t" << *FirstTerminator; errs() << "First terminator was:\t" << *FirstTerminator;
} }
} }
}
// The operands on an INLINEASM instruction must follow a template. // The operands on an INLINEASM instruction must follow a template.
// Verify that the flag operands make sense. // Verify that the flag operands make sense.
void MachineVerifier::verifyInlineAsm(const MachineInstr *MI) { void MachineVerifier::verifyInlineAsm(const MachineInstr *MI) {
// The first two operands on INLINEASM are the asm string and global flags. // The first two operands on INLINEASM are the asm string and global flags.
if (MI->getNumOperands() < 2) { if (MI->getNumOperands() < 2) {
report("Too few operands on inline asm", MI); report("Too few operands on inline asm", MI);
return; return;
▲ Show 20 Lines • Show All 660 Lines • ▼ Show 20 Linesvoid MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
} }
StringRef ErrorInfo; StringRef ErrorInfo;
if (!TII->verifyInstruction(*MI, ErrorInfo)) if (!TII->verifyInstruction(*MI, ErrorInfo))
report(ErrorInfo.data(), MI); report(ErrorInfo.data(), MI);
// Verify properties of various specific instruction types // Verify properties of various specific instruction types
switch (MI->getOpcode()) { switch (MI->getOpcode()) {
case TargetOpcode::TCOPY: {
MachineBasicBlock::const_iterator MII(MI), MIE = MI->getParent()->end();
for (; MII != MIE; ++MII) {
if (MII->getOpcode() != TargetOpcode::COPY)
continue;
report("TCOPY and COPY instructions are intermixed", &*MII);
errs() << "- TCOPY instruction: ";
if (Indexes && Indexes->hasIndex(*MI))
errs() << Indexes->getInstructionIndex(*MI) << '\t';
MI->print(errs(), /*SkipOpers=*/true);
}
LLVM_FALLTHROUGH;
}
case TargetOpcode::COPY: { case TargetOpcode::COPY: {
if (foundErrors) if (foundErrors)
break; break;
const MachineOperand &DstOp = MI->getOperand(0); const MachineOperand &DstOp = MI->getOperand(0);
const MachineOperand &SrcOp = MI->getOperand(1); const MachineOperand &SrcOp = MI->getOperand(1);
LLT DstTy = MRI->getType(DstOp.getReg()); LLT DstTy = MRI->getType(DstOp.getReg());
LLT SrcTy = MRI->getType(SrcOp.getReg()); LLT SrcTy = MRI->getType(SrcOp.getReg());
if (SrcTy.isValid() && DstTy.isValid()) { if (SrcTy.isValid() && DstTy.isValid()) {
▲ Show 20 Lines • Show All 1,453 Lines • Show Last 20 Lines