diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp index b7218afdd382066..e2c09fe25d55cd4 100644 --- a/llvm/lib/CodeGen/MachineVerifier.cpp +++ b/llvm/lib/CodeGen/MachineVerifier.cpp @@ -77,8 +77,10 @@ #include "llvm/Pass.h" #include "llvm/Support/Casting.h" #include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/ManagedStatic.h" #include "llvm/Support/MathExtras.h" #include "llvm/Support/ModRef.h" +#include "llvm/Support/Mutex.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Target/TargetMachine.h" #include @@ -93,21 +95,31 @@ using namespace llvm; namespace { +/// Used the by the ReportedErrors class to guarantee only one error is reported +/// at one time. +static ManagedStatic> ReportedErrorsLock; + struct MachineVerifier { MachineVerifier(MachineFunctionAnalysisManager &MFAM, const char *b, - raw_ostream *OS) - : MFAM(&MFAM), OS(OS ? *OS : nulls()), Banner(b) {} + raw_ostream *OS, bool AbortOnError = true) + : MFAM(&MFAM), OS(OS ? *OS : nulls()), Banner(b), + ReportedErrs(AbortOnError) {} - MachineVerifier(Pass *pass, const char *b, raw_ostream *OS) - : PASS(pass), OS(OS ? *OS : nulls()), Banner(b) {} + MachineVerifier(Pass *pass, const char *b, raw_ostream *OS, + bool AbortOnError = true) + : PASS(pass), OS(OS ? *OS : nulls()), Banner(b), + ReportedErrs(AbortOnError) {} MachineVerifier(const char *b, LiveVariables *LiveVars, LiveIntervals *LiveInts, LiveStacks *LiveStks, - SlotIndexes *Indexes, raw_ostream *OS) + SlotIndexes *Indexes, raw_ostream *OS, + bool AbortOnError = true) : OS(OS ? *OS : nulls()), Banner(b), LiveVars(LiveVars), - LiveInts(LiveInts), LiveStks(LiveStks), Indexes(Indexes) {} + LiveInts(LiveInts), LiveStks(LiveStks), Indexes(Indexes), + ReportedErrs(AbortOnError) {} - unsigned verify(const MachineFunction &MF); + /// \returns true if no problems were found. + bool verify(const MachineFunction &MF); MachineFunctionAnalysisManager *MFAM = nullptr; Pass *const PASS = nullptr; @@ -120,8 +132,6 @@ struct MachineVerifier { const MachineRegisterInfo *MRI = nullptr; const RegisterBankInfo *RBI = nullptr; - unsigned foundErrors = 0; - // Avoid querying the MachineFunctionProperties for each operand. bool isFunctionRegBankSelected = false; bool isFunctionSelected = false; @@ -231,6 +241,44 @@ struct MachineVerifier { LiveStacks *LiveStks = nullptr; SlotIndexes *Indexes = nullptr; + /// A class to track the number of reported error and to guarantee that only + /// one error is reported at one time. + class ReportedErrors { + unsigned NumReported = 0; + bool AbortOnError; + + public: + /// \param AbortOnError -- If set, abort after printing the first error. + ReportedErrors(bool AbortOnError) : AbortOnError(AbortOnError) {} + + ~ReportedErrors() { + if (!hasError()) + return; + if (AbortOnError) + report_fatal_error("Found " + Twine(NumReported) + + " machine code errors."); + // Since we haven't aborted, release the lock to allow other threads to + // report errors. + ReportedErrorsLock->unlock(); + } + + /// Increment the number of reported errors. + /// \returns true if this is the first reported error. + bool increment() { + // If this is the first error this thread has encountered, grab the lock + // to prevent other threads from reporting errors at the same time. + // Otherwise we assume we already have the lock. + if (!hasError()) + ReportedErrorsLock->lock(); + ++NumReported; + return NumReported == 1; + } + + /// \returns true if an error was reported. + bool hasError() { return NumReported; } + }; + ReportedErrors ReportedErrs; + // This is calculated only when trying to verify convergence control tokens. // Similar to the LLVM IR verifier, we calculate this locally instead of // relying on the pass manager. @@ -337,11 +385,7 @@ struct MachineVerifierLegacyPass : public MachineFunctionPass { MachineFunctionProperties::Property::FailsVerification)) return false; - unsigned FoundErrors = - MachineVerifier(this, Banner.c_str(), &errs()).verify(MF); - if (FoundErrors) - report_fatal_error("Found " + Twine(FoundErrors) + - " machine code errors."); + MachineVerifier(this, Banner.c_str(), &errs()).verify(MF); return false; } }; @@ -357,10 +401,7 @@ MachineVerifierPass::run(MachineFunction &MF, if (MF.getProperties().hasProperty( MachineFunctionProperties::Property::FailsVerification)) return PreservedAnalyses::all(); - unsigned FoundErrors = - MachineVerifier(MFAM, Banner.c_str(), &errs()).verify(MF); - if (FoundErrors) - report_fatal_error("Found " + Twine(FoundErrors) + " machine code errors."); + MachineVerifier(MFAM, Banner.c_str(), &errs()).verify(MF); return PreservedAnalyses::all(); } @@ -380,31 +421,20 @@ void llvm::verifyMachineFunction(const std::string &Banner, // LiveIntervals *LiveInts; // LiveStacks *LiveStks; // SlotIndexes *Indexes; - unsigned FoundErrors = - MachineVerifier(nullptr, Banner.c_str(), &errs()).verify(MF); - if (FoundErrors) - report_fatal_error("Found " + Twine(FoundErrors) + " machine code errors."); + MachineVerifier(nullptr, Banner.c_str(), &errs()).verify(MF); } bool MachineFunction::verify(Pass *p, const char *Banner, raw_ostream *OS, - bool AbortOnErrors) const { - MachineFunction &MF = const_cast(*this); - unsigned FoundErrors = MachineVerifier(p, Banner, OS).verify(MF); - if (AbortOnErrors && FoundErrors) - report_fatal_error("Found "+Twine(FoundErrors)+" machine code errors."); - return FoundErrors == 0; + bool AbortOnError) const { + return MachineVerifier(p, Banner, OS, AbortOnError).verify(*this); } bool MachineFunction::verify(LiveIntervals *LiveInts, SlotIndexes *Indexes, const char *Banner, raw_ostream *OS, - bool AbortOnErrors) const { - MachineFunction &MF = const_cast(*this); - unsigned FoundErrors = - MachineVerifier(Banner, nullptr, LiveInts, nullptr, Indexes, OS) - .verify(MF); - if (AbortOnErrors && FoundErrors) - report_fatal_error("Found " + Twine(FoundErrors) + " machine code errors."); - return FoundErrors == 0; + bool AbortOnError) const { + return MachineVerifier(Banner, /*LiveVars=*/nullptr, LiveInts, + /*LiveStks=*/nullptr, Indexes, OS, AbortOnError) + .verify(*this); } void MachineVerifier::verifySlotIndexes() const { @@ -430,9 +460,7 @@ void MachineVerifier::verifyProperties(const MachineFunction &MF) { report("Function has NoVRegs property but there are VReg operands", &MF); } -unsigned MachineVerifier::verify(const MachineFunction &MF) { - foundErrors = 0; - +bool MachineVerifier::verify(const MachineFunction &MF) { this->MF = &MF; TM = &MF.getTarget(); TII = MF.getSubtarget().getInstrInfo(); @@ -447,7 +475,7 @@ unsigned MachineVerifier::verify(const MachineFunction &MF) { // it's expected that the MIR is somewhat broken but that's ok since we'll // reset it and clear the FailedISel attribute in ResetMachineFunctions. if (isFunctionFailedISel) - return foundErrors; + return true; isFunctionRegBankSelected = MF.getProperties().hasProperty( MachineFunctionProperties::Property::RegBankSelected); @@ -544,13 +572,13 @@ unsigned MachineVerifier::verify(const MachineFunction &MF) { regMasks.clear(); MBBInfoMap.clear(); - return foundErrors; + return !ReportedErrs.hasError(); } void MachineVerifier::report(const char *msg, const MachineFunction *MF) { assert(MF); OS << '\n'; - if (!foundErrors++) { + if (ReportedErrs.increment()) { if (Banner) OS << "# " << Banner << '\n'; diff --git a/llvm/test/MachineVerifier/test_multiple_errors.mir b/llvm/test/MachineVerifier/test_multiple_errors.mir new file mode 100644 index 000000000000000..e1ce348565c1aa6 --- /dev/null +++ b/llvm/test/MachineVerifier/test_multiple_errors.mir @@ -0,0 +1,22 @@ +# RUN: not --crash llc -mtriple=aarch64 -o /dev/null -run-pass=none %s 2>&1 | FileCheck %s --implicit-check-not="Bad machine code" +# REQUIRES: aarch64-registered-target + +# Since we abort after reporting the first error, we should only expect one error to be reported. +# CHECK: *** Bad machine code: Generic virtual register use cannot be undef *** +# CHECK: Found 1 machine code errors. + +--- +name: foo +liveins: +body: | + bb.0: + $x0 = COPY undef %0:_(s64) +... + +--- +name: bar +liveins: +body: | + bb.0: + $x0 = COPY undef %0:_(s64) +...