[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 17:01:30 PDT 2024


https://github.com/ellishg updated https://github.com/llvm/llvm-project/pull/111605

>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 985d7c39bdc16d87120538da88e3168e4219d0c6 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..a39d364b1c02bd 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 = 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 +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