[llvm] adaa603 - [MachineVerifier] Report errors from one thread at a time (#111605)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 11 11:53:48 PDT 2024
Author: Ellis Hoag
Date: 2024-10-11T11:53:44-07:00
New Revision: adaa603224fee842b4f71824617adb3b0c7d9cf1
URL: https://github.com/llvm/llvm-project/commit/adaa603224fee842b4f71824617adb3b0c7d9cf1
DIFF: https://github.com/llvm/llvm-project/commit/adaa603224fee842b4f71824617adb3b0c7d9cf1.diff
LOG: [MachineVerifier] Report errors from one thread at a time (#111605)
Create the `ReportedErrors` class to track the number of reported errors
during verification. The class will block reporting errors if some other
thread is currently reporting an error.
I've encountered a case where there were many different verifications
reporting errors at the same time on different threads. This ensures
that we don't start printing the error from one case until we are
completely done printing errors from other cases. Most of the time
`AbortOnError = true` so we usually abort after reporting the first
error.
Depends on https://github.com/llvm/llvm-project/pull/111602.
Added:
llvm/test/MachineVerifier/test_multiple_errors.mir
Modified:
llvm/lib/CodeGen/MachineVerifier.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index b7218afdd38206..e2c09fe25d55cd 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 <algorithm>
@@ -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<sys::SmartMutex<true>> 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<MachineFunction&>(*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<MachineFunction &>(*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 00000000000000..e1ce348565c1aa
--- /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)
+...
More information about the llvm-commits
mailing list