[llvm] [MachineVerifier] Report errors from one thread at a time (PR #111605)
Ellis Hoag via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 8 16:33:52 PDT 2024
https://github.com/ellishg created https://github.com/llvm/llvm-project/pull/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.
>From af4a68e21a434cfba3cca7654705648d1c382d87 Mon Sep 17 00:00:00 2001
From: Ellis Hoag <ellis.sparky.hoag at gmail.com>
Date: Tue, 8 Oct 2024 15:54:35 -0700
Subject: [PATCH 1/2] Format MachineVerifier.cpp
---
llvm/lib/CodeGen/MachineVerifier.cpp | 501 ++++++++++++++-------------
1 file changed, 251 insertions(+), 250 deletions(-)
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 24a0f41775cc1d..b7218afdd38206 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -93,257 +93,258 @@ using namespace llvm;
namespace {
- struct MachineVerifier {
- MachineVerifier(MachineFunctionAnalysisManager &MFAM, const char *b,
- raw_ostream *OS)
- : MFAM(&MFAM), OS(OS ? *OS : nulls()), Banner(b) {}
-
- MachineVerifier(Pass *pass, const char *b, raw_ostream *OS)
- : PASS(pass), OS(OS ? *OS : nulls()), Banner(b) {}
-
- MachineVerifier(const char *b, LiveVariables *LiveVars,
- LiveIntervals *LiveInts, LiveStacks *LiveStks,
- SlotIndexes *Indexes, raw_ostream *OS)
- : OS(OS ? *OS : nulls()), Banner(b), LiveVars(LiveVars),
- LiveInts(LiveInts), LiveStks(LiveStks), Indexes(Indexes) {}
-
- unsigned verify(const MachineFunction &MF);
-
- MachineFunctionAnalysisManager *MFAM = nullptr;
- Pass *const PASS = nullptr;
- raw_ostream &OS;
- const char *Banner;
- const MachineFunction *MF = nullptr;
- const TargetMachine *TM = nullptr;
- const TargetInstrInfo *TII = nullptr;
- const TargetRegisterInfo *TRI = nullptr;
- const MachineRegisterInfo *MRI = nullptr;
- const RegisterBankInfo *RBI = nullptr;
-
- unsigned foundErrors = 0;
-
- // Avoid querying the MachineFunctionProperties for each operand.
- bool isFunctionRegBankSelected = false;
- bool isFunctionSelected = false;
- bool isFunctionTracksDebugUserValues = false;
-
- using RegVector = SmallVector<Register, 16>;
- using RegMaskVector = SmallVector<const uint32_t *, 4>;
- using RegSet = DenseSet<Register>;
- using RegMap = DenseMap<Register, const MachineInstr *>;
- using BlockSet = SmallPtrSet<const MachineBasicBlock *, 8>;
-
- const MachineInstr *FirstNonPHI = nullptr;
- const MachineInstr *FirstTerminator = nullptr;
- BlockSet FunctionBlocks;
-
- BitVector regsReserved;
- RegSet regsLive;
- RegVector regsDefined, regsDead, regsKilled;
- RegMaskVector regMasks;
-
- SlotIndex lastIndex;
-
- // Add Reg and any sub-registers to RV
- void addRegWithSubRegs(RegVector &RV, Register Reg) {
- RV.push_back(Reg);
- if (Reg.isPhysical())
- append_range(RV, TRI->subregs(Reg.asMCReg()));
- }
-
- struct BBInfo {
- // Is this MBB reachable from the MF entry point?
- bool reachable = false;
-
- // Vregs that must be live in because they are used without being
- // defined. Map value is the user. vregsLiveIn doesn't include regs
- // that only are used by PHI nodes.
- RegMap vregsLiveIn;
-
- // Regs killed in MBB. They may be defined again, and will then be in both
- // regsKilled and regsLiveOut.
- RegSet regsKilled;
-
- // Regs defined in MBB and live out. Note that vregs passing through may
- // be live out without being mentioned here.
- RegSet regsLiveOut;
-
- // Vregs that pass through MBB untouched. This set is disjoint from
- // regsKilled and regsLiveOut.
- RegSet vregsPassed;
-
- // Vregs that must pass through MBB because they are needed by a successor
- // block. This set is disjoint from regsLiveOut.
- RegSet vregsRequired;
-
- // Set versions of block's predecessor and successor lists.
- BlockSet Preds, Succs;
-
- BBInfo() = default;
-
- // Add register to vregsRequired if it belongs there. Return true if
- // anything changed.
- bool addRequired(Register Reg) {
- if (!Reg.isVirtual())
- return false;
- if (regsLiveOut.count(Reg))
- return false;
- return vregsRequired.insert(Reg).second;
- }
+struct MachineVerifier {
+ MachineVerifier(MachineFunctionAnalysisManager &MFAM, const char *b,
+ raw_ostream *OS)
+ : MFAM(&MFAM), OS(OS ? *OS : nulls()), Banner(b) {}
+
+ MachineVerifier(Pass *pass, const char *b, raw_ostream *OS)
+ : PASS(pass), OS(OS ? *OS : nulls()), Banner(b) {}
+
+ MachineVerifier(const char *b, LiveVariables *LiveVars,
+ LiveIntervals *LiveInts, LiveStacks *LiveStks,
+ SlotIndexes *Indexes, raw_ostream *OS)
+ : OS(OS ? *OS : nulls()), Banner(b), LiveVars(LiveVars),
+ LiveInts(LiveInts), LiveStks(LiveStks), Indexes(Indexes) {}
+
+ unsigned verify(const MachineFunction &MF);
+
+ MachineFunctionAnalysisManager *MFAM = nullptr;
+ Pass *const PASS = nullptr;
+ raw_ostream &OS;
+ const char *Banner;
+ const MachineFunction *MF = nullptr;
+ const TargetMachine *TM = nullptr;
+ const TargetInstrInfo *TII = nullptr;
+ const TargetRegisterInfo *TRI = nullptr;
+ const MachineRegisterInfo *MRI = nullptr;
+ const RegisterBankInfo *RBI = nullptr;
+
+ unsigned foundErrors = 0;
+
+ // Avoid querying the MachineFunctionProperties for each operand.
+ bool isFunctionRegBankSelected = false;
+ bool isFunctionSelected = false;
+ bool isFunctionTracksDebugUserValues = false;
+
+ using RegVector = SmallVector<Register, 16>;
+ using RegMaskVector = SmallVector<const uint32_t *, 4>;
+ using RegSet = DenseSet<Register>;
+ using RegMap = DenseMap<Register, const MachineInstr *>;
+ using BlockSet = SmallPtrSet<const MachineBasicBlock *, 8>;
+
+ const MachineInstr *FirstNonPHI = nullptr;
+ const MachineInstr *FirstTerminator = nullptr;
+ BlockSet FunctionBlocks;
+
+ BitVector regsReserved;
+ RegSet regsLive;
+ RegVector regsDefined, regsDead, regsKilled;
+ RegMaskVector regMasks;
+
+ SlotIndex lastIndex;
+
+ // Add Reg and any sub-registers to RV
+ void addRegWithSubRegs(RegVector &RV, Register Reg) {
+ RV.push_back(Reg);
+ if (Reg.isPhysical())
+ append_range(RV, TRI->subregs(Reg.asMCReg()));
+ }
+
+ struct BBInfo {
+ // Is this MBB reachable from the MF entry point?
+ bool reachable = false;
+
+ // Vregs that must be live in because they are used without being
+ // defined. Map value is the user. vregsLiveIn doesn't include regs
+ // that only are used by PHI nodes.
+ RegMap vregsLiveIn;
+
+ // Regs killed in MBB. They may be defined again, and will then be in both
+ // regsKilled and regsLiveOut.
+ RegSet regsKilled;
+
+ // Regs defined in MBB and live out. Note that vregs passing through may
+ // be live out without being mentioned here.
+ RegSet regsLiveOut;
+
+ // Vregs that pass through MBB untouched. This set is disjoint from
+ // regsKilled and regsLiveOut.
+ RegSet vregsPassed;
+
+ // Vregs that must pass through MBB because they are needed by a successor
+ // block. This set is disjoint from regsLiveOut.
+ RegSet vregsRequired;
+
+ // Set versions of block's predecessor and successor lists.
+ BlockSet Preds, Succs;
+
+ BBInfo() = default;
+
+ // Add register to vregsRequired if it belongs there. Return true if
+ // anything changed.
+ bool addRequired(Register Reg) {
+ if (!Reg.isVirtual())
+ return false;
+ if (regsLiveOut.count(Reg))
+ return false;
+ return vregsRequired.insert(Reg).second;
+ }
- // Same for a full set.
- bool addRequired(const RegSet &RS) {
- bool Changed = false;
- for (Register Reg : RS)
- Changed |= addRequired(Reg);
- return Changed;
- }
+ // Same for a full set.
+ bool addRequired(const RegSet &RS) {
+ bool Changed = false;
+ for (Register Reg : RS)
+ Changed |= addRequired(Reg);
+ return Changed;
+ }
- // Same for a full map.
- bool addRequired(const RegMap &RM) {
- bool Changed = false;
- for (const auto &I : RM)
- Changed |= addRequired(I.first);
- return Changed;
- }
+ // Same for a full map.
+ bool addRequired(const RegMap &RM) {
+ bool Changed = false;
+ for (const auto &I : RM)
+ Changed |= addRequired(I.first);
+ return Changed;
+ }
- // Live-out registers are either in regsLiveOut or vregsPassed.
- bool isLiveOut(Register Reg) const {
- return regsLiveOut.count(Reg) || vregsPassed.count(Reg);
- }
- };
+ // Live-out registers are either in regsLiveOut or vregsPassed.
+ bool isLiveOut(Register Reg) const {
+ return regsLiveOut.count(Reg) || vregsPassed.count(Reg);
+ }
+ };
- // Extra register info per MBB.
- DenseMap<const MachineBasicBlock*, BBInfo> MBBInfoMap;
-
- bool isReserved(Register Reg) {
- return Reg.id() < regsReserved.size() && regsReserved.test(Reg.id());
- }
-
- bool isAllocatable(Register Reg) const {
- return Reg.id() < TRI->getNumRegs() && TRI->isInAllocatableClass(Reg) &&
- !regsReserved.test(Reg.id());
- }
-
- // Analysis information if available
- LiveVariables *LiveVars = nullptr;
- LiveIntervals *LiveInts = nullptr;
- LiveStacks *LiveStks = nullptr;
- SlotIndexes *Indexes = nullptr;
-
- // 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.
- MachineDominatorTree DT;
-
- void visitMachineFunctionBefore();
- void visitMachineBasicBlockBefore(const MachineBasicBlock *MBB);
- void visitMachineBundleBefore(const MachineInstr *MI);
-
- /// Verify that all of \p MI's virtual register operands are scalars.
- /// \returns True if all virtual register operands are scalar. False
- /// otherwise.
- bool verifyAllRegOpsScalar(const MachineInstr &MI,
- const MachineRegisterInfo &MRI);
- bool verifyVectorElementMatch(LLT Ty0, LLT Ty1, const MachineInstr *MI);
-
- bool verifyGIntrinsicSideEffects(const MachineInstr *MI);
- bool verifyGIntrinsicConvergence(const MachineInstr *MI);
- void verifyPreISelGenericInstruction(const MachineInstr *MI);
-
- void visitMachineInstrBefore(const MachineInstr *MI);
- void visitMachineOperand(const MachineOperand *MO, unsigned MONum);
- void visitMachineBundleAfter(const MachineInstr *MI);
- void visitMachineBasicBlockAfter(const MachineBasicBlock *MBB);
- void visitMachineFunctionAfter();
-
- void report(const char *msg, const MachineFunction *MF);
- void report(const char *msg, const MachineBasicBlock *MBB);
- void report(const char *msg, const MachineInstr *MI);
- void report(const char *msg, const MachineOperand *MO, unsigned MONum,
- LLT MOVRegType = LLT{});
- void report(const Twine &Msg, const MachineInstr *MI);
-
- void report_context(const LiveInterval &LI) const;
- void report_context(const LiveRange &LR, Register VRegUnit,
- LaneBitmask LaneMask) const;
- void report_context(const LiveRange::Segment &S) const;
- void report_context(const VNInfo &VNI) const;
- void report_context(SlotIndex Pos) const;
- void report_context(MCPhysReg PhysReg) const;
- void report_context_liverange(const LiveRange &LR) const;
- void report_context_lanemask(LaneBitmask LaneMask) const;
- void report_context_vreg(Register VReg) const;
- void report_context_vreg_regunit(Register VRegOrUnit) const;
-
- void verifyInlineAsm(const MachineInstr *MI);
-
- void checkLiveness(const MachineOperand *MO, unsigned MONum);
- void checkLivenessAtUse(const MachineOperand *MO, unsigned MONum,
- SlotIndex UseIdx, const LiveRange &LR,
- Register VRegOrUnit,
- LaneBitmask LaneMask = LaneBitmask::getNone());
- void checkLivenessAtDef(const MachineOperand *MO, unsigned MONum,
- SlotIndex DefIdx, const LiveRange &LR,
- Register VRegOrUnit, bool SubRangeCheck = false,
- LaneBitmask LaneMask = LaneBitmask::getNone());
-
- void markReachable(const MachineBasicBlock *MBB);
- void calcRegsPassed();
- void checkPHIOps(const MachineBasicBlock &MBB);
-
- void calcRegsRequired();
- void verifyLiveVariables();
- void verifyLiveIntervals();
- void verifyLiveInterval(const LiveInterval&);
- void verifyLiveRangeValue(const LiveRange &, const VNInfo *, Register,
+ // Extra register info per MBB.
+ DenseMap<const MachineBasicBlock *, BBInfo> MBBInfoMap;
+
+ bool isReserved(Register Reg) {
+ return Reg.id() < regsReserved.size() && regsReserved.test(Reg.id());
+ }
+
+ bool isAllocatable(Register Reg) const {
+ return Reg.id() < TRI->getNumRegs() && TRI->isInAllocatableClass(Reg) &&
+ !regsReserved.test(Reg.id());
+ }
+
+ // Analysis information if available
+ LiveVariables *LiveVars = nullptr;
+ LiveIntervals *LiveInts = nullptr;
+ LiveStacks *LiveStks = nullptr;
+ SlotIndexes *Indexes = nullptr;
+
+ // 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.
+ MachineDominatorTree DT;
+
+ void visitMachineFunctionBefore();
+ void visitMachineBasicBlockBefore(const MachineBasicBlock *MBB);
+ void visitMachineBundleBefore(const MachineInstr *MI);
+
+ /// Verify that all of \p MI's virtual register operands are scalars.
+ /// \returns True if all virtual register operands are scalar. False
+ /// otherwise.
+ bool verifyAllRegOpsScalar(const MachineInstr &MI,
+ const MachineRegisterInfo &MRI);
+ bool verifyVectorElementMatch(LLT Ty0, LLT Ty1, const MachineInstr *MI);
+
+ bool verifyGIntrinsicSideEffects(const MachineInstr *MI);
+ bool verifyGIntrinsicConvergence(const MachineInstr *MI);
+ void verifyPreISelGenericInstruction(const MachineInstr *MI);
+
+ void visitMachineInstrBefore(const MachineInstr *MI);
+ void visitMachineOperand(const MachineOperand *MO, unsigned MONum);
+ void visitMachineBundleAfter(const MachineInstr *MI);
+ void visitMachineBasicBlockAfter(const MachineBasicBlock *MBB);
+ void visitMachineFunctionAfter();
+
+ void report(const char *msg, const MachineFunction *MF);
+ void report(const char *msg, const MachineBasicBlock *MBB);
+ void report(const char *msg, const MachineInstr *MI);
+ void report(const char *msg, const MachineOperand *MO, unsigned MONum,
+ LLT MOVRegType = LLT{});
+ void report(const Twine &Msg, const MachineInstr *MI);
+
+ void report_context(const LiveInterval &LI) const;
+ void report_context(const LiveRange &LR, Register VRegUnit,
+ LaneBitmask LaneMask) const;
+ void report_context(const LiveRange::Segment &S) const;
+ void report_context(const VNInfo &VNI) const;
+ void report_context(SlotIndex Pos) const;
+ void report_context(MCPhysReg PhysReg) const;
+ void report_context_liverange(const LiveRange &LR) const;
+ void report_context_lanemask(LaneBitmask LaneMask) const;
+ void report_context_vreg(Register VReg) const;
+ void report_context_vreg_regunit(Register VRegOrUnit) const;
+
+ void verifyInlineAsm(const MachineInstr *MI);
+
+ void checkLiveness(const MachineOperand *MO, unsigned MONum);
+ void checkLivenessAtUse(const MachineOperand *MO, unsigned MONum,
+ SlotIndex UseIdx, const LiveRange &LR,
+ Register VRegOrUnit,
+ LaneBitmask LaneMask = LaneBitmask::getNone());
+ void checkLivenessAtDef(const MachineOperand *MO, unsigned MONum,
+ SlotIndex DefIdx, const LiveRange &LR,
+ Register VRegOrUnit, bool SubRangeCheck = false,
+ LaneBitmask LaneMask = LaneBitmask::getNone());
+
+ void markReachable(const MachineBasicBlock *MBB);
+ void calcRegsPassed();
+ void checkPHIOps(const MachineBasicBlock &MBB);
+
+ void calcRegsRequired();
+ void verifyLiveVariables();
+ void verifyLiveIntervals();
+ void verifyLiveInterval(const LiveInterval &);
+ void verifyLiveRangeValue(const LiveRange &, const VNInfo *, Register,
+ LaneBitmask);
+ void verifyLiveRangeSegment(const LiveRange &,
+ const LiveRange::const_iterator I, Register,
LaneBitmask);
- void verifyLiveRangeSegment(const LiveRange &,
- const LiveRange::const_iterator I, Register,
- LaneBitmask);
- void verifyLiveRange(const LiveRange &, Register,
- LaneBitmask LaneMask = LaneBitmask::getNone());
+ void verifyLiveRange(const LiveRange &, Register,
+ LaneBitmask LaneMask = LaneBitmask::getNone());
- void verifyStackFrame();
+ void verifyStackFrame();
- void verifySlotIndexes() const;
- void verifyProperties(const MachineFunction &MF);
- };
-
- struct MachineVerifierLegacyPass : public MachineFunctionPass {
- static char ID; // Pass ID, replacement for typeid
+ void verifySlotIndexes() const;
+ void verifyProperties(const MachineFunction &MF);
+};
- const std::string Banner;
+struct MachineVerifierLegacyPass : public MachineFunctionPass {
+ static char ID; // Pass ID, replacement for typeid
- MachineVerifierLegacyPass(std::string banner = std::string())
- : MachineFunctionPass(ID), Banner(std::move(banner)) {
- initializeMachineVerifierLegacyPassPass(*PassRegistry::getPassRegistry());
- }
+ const std::string Banner;
- void getAnalysisUsage(AnalysisUsage &AU) const override {
- AU.addUsedIfAvailable<LiveStacks>();
- AU.addUsedIfAvailable<LiveVariablesWrapperPass>();
- AU.addUsedIfAvailable<SlotIndexesWrapperPass>();
- AU.addUsedIfAvailable<LiveIntervalsWrapperPass>();
- AU.setPreservesAll();
- MachineFunctionPass::getAnalysisUsage(AU);
- }
+ MachineVerifierLegacyPass(std::string banner = std::string())
+ : MachineFunctionPass(ID), Banner(std::move(banner)) {
+ initializeMachineVerifierLegacyPassPass(*PassRegistry::getPassRegistry());
+ }
- bool runOnMachineFunction(MachineFunction &MF) override {
- // Skip functions that have known verification problems.
- // FIXME: Remove this mechanism when all problematic passes have been
- // fixed.
- if (MF.getProperties().hasProperty(
- MachineFunctionProperties::Property::FailsVerification))
- return false;
+ void getAnalysisUsage(AnalysisUsage &AU) const override {
+ AU.addUsedIfAvailable<LiveStacks>();
+ AU.addUsedIfAvailable<LiveVariablesWrapperPass>();
+ AU.addUsedIfAvailable<SlotIndexesWrapperPass>();
+ AU.addUsedIfAvailable<LiveIntervalsWrapperPass>();
+ AU.setPreservesAll();
+ MachineFunctionPass::getAnalysisUsage(AU);
+ }
- unsigned FoundErrors =
- MachineVerifier(this, Banner.c_str(), &errs()).verify(MF);
- if (FoundErrors)
- report_fatal_error("Found "+Twine(FoundErrors)+" machine code errors.");
+ bool runOnMachineFunction(MachineFunction &MF) override {
+ // Skip functions that have known verification problems.
+ // FIXME: Remove this mechanism when all problematic passes have been
+ // fixed.
+ if (MF.getProperties().hasProperty(
+ 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.");
+ return false;
+ }
+};
} // end anonymous namespace
@@ -3846,18 +3847,18 @@ namespace {
// integer, we can't tell whether it is a FrameSetup or FrameDestroy if the
// value is zero.
// We use a bool plus an integer to capture the stack state.
- struct StackStateOfBB {
- StackStateOfBB() = default;
- StackStateOfBB(int EntryVal, int ExitVal, bool EntrySetup, bool ExitSetup) :
- EntryValue(EntryVal), ExitValue(ExitVal), EntryIsSetup(EntrySetup),
- ExitIsSetup(ExitSetup) {}
-
- // Can be negative, which means we are setting up a frame.
- int EntryValue = 0;
- int ExitValue = 0;
- bool EntryIsSetup = false;
- bool ExitIsSetup = false;
- };
+struct StackStateOfBB {
+ StackStateOfBB() = default;
+ StackStateOfBB(int EntryVal, int ExitVal, bool EntrySetup, bool ExitSetup)
+ : EntryValue(EntryVal), ExitValue(ExitVal), EntryIsSetup(EntrySetup),
+ ExitIsSetup(ExitSetup) {}
+
+ // Can be negative, which means we are setting up a frame.
+ int EntryValue = 0;
+ int ExitValue = 0;
+ bool EntryIsSetup = false;
+ bool ExitIsSetup = false;
+};
} // end anonymous namespace
>From bd7f0ea2b20761eee94ddc310f8b736e4bbda196 Mon Sep 17 00:00:00 2001
From: Ellis Hoag <ellis.sparky.hoag at gmail.com>
Date: Tue, 8 Oct 2024 15:57:29 -0700
Subject: [PATCH 2/2] [MachineVerifier] Report errors from one thread at a time
---
llvm/lib/CodeGen/MachineVerifier.cpp | 105 +++++++++++-------
.../X86/machine-verifier-address-threads.mir | 55 +++++++++
2 files changed, 118 insertions(+), 42 deletions(-)
create mode 100644 llvm/test/CodeGen/MIR/X86/machine-verifier-address-threads.mir
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index b7218afdd38206..5dad6cdc854b8f 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,29 @@ using namespace llvm;
namespace {
+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 = false)
+ : 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 = false)
+ : 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 = false)
: 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 +130,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 +239,39 @@ struct MachineVerifier {
LiveStacks *LiveStks = nullptr;
SlotIndexes *Indexes = nullptr;
+ class ReportedErrors {
+ unsigned NumReported = 0;
+ bool AbortOnError;
+
+ public:
+ 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;
+ }
+
+ 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 +378,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 +394,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 +414,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 +453,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 +468,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 +565,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/CodeGen/MIR/X86/machine-verifier-address-threads.mir b/llvm/test/CodeGen/MIR/X86/machine-verifier-address-threads.mir
new file mode 100644
index 00000000000000..5d4e73d8deb328
--- /dev/null
+++ b/llvm/test/CodeGen/MIR/X86/machine-verifier-address-threads.mir
@@ -0,0 +1,55 @@
+# RUN: not --crash llc -march=x86-64 -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
+# This test ensures that the address is checked in machine verifier.
+
+---
+name: baz
+tracksRegLiveness: true
+body: |
+ bb.0:
+ successors: %bb.1(0x80000000)
+ liveins: $rdi, $xmm0
+
+ %1:vr128 = COPY $xmm0
+ %0:gr64 = COPY $rdi
+ %2:vr128 = COPY %1
+
+ bb.1:
+ successors: %bb.1(0x80000000)
+
+ %3:vr256 = AVX_SET0
+ %4:vr128 = VPSLLDri %2, 31
+ %5:vr256 = VPMOVSXDQYrr killed %4
+ %8:vr256 = IMPLICIT_DEF
+ %6:vr256, %7:vr256 = VGATHERQPDYrm %3, %0, 16, killed %8, 0, $noreg, %5 :: (load unknown-size, align 8)
+ %9:vr128 = COPY %6.sub_xmm
+ VMOVLPDmr $noreg, 1, $noreg, 1111111111111, $noreg, killed %9 :: (store (s64) into `ptr undef`)
+ JMP_1 %bb.1
+
+...
+---
+name: bar
+tracksRegLiveness: true
+body: |
+ bb.0:
+ successors: %bb.1(0x80000000)
+ liveins: $rdi, $xmm0
+
+ %1:vr128 = COPY $xmm0
+ %0:gr64 = COPY $rdi
+ %2:vr128 = COPY %1
+
+ bb.1:
+ successors: %bb.1(0x80000000)
+
+ %3:vr256 = AVX_SET0
+ %4:vr128 = VPSLLDri %2, 31
+ %5:vr256 = VPMOVSXDQYrr killed %4
+ %8:vr256 = IMPLICIT_DEF
+ %6:vr256, %7:vr256 = VGATHERQPDYrm %3, %0, 16, killed %8, 0, $noreg, %5 :: (load unknown-size, align 8)
+ %9:vr128 = COPY %6.sub_xmm
+ VMOVLPDmr $noreg, 1, $noreg, 1111111111111, $noreg, killed %9 :: (store (s64) into `ptr undef`)
+ JMP_1 %bb.1
+
+...
+
+; CHECK: LLVM ERROR: Found 2 machine code errors
More information about the llvm-commits
mailing list