[llvm-mca][RISCV] Fix llvm-mca RISCVInstrument memory leak

[michaelmaitland]

Summary

There was a memory leak that presented itself once the llvm-mca
tests were committed. This leak was not checked for by the pre-commit
tests. This change changes the shared_ptr to a unique_ptr to avoid
this problem.

We will know that this fix works once committed since I don't know
whether it is possible to force a lit test to use LSan. I spent the
day trying to build llvm with LSan enabled without much luck. If
anyone knows how to build llvm with LSan for the lit-tests, I am
happy to give it another try locally.

[michaelmaitland]

Update docstring.

[craig.topper]

llvm/tools/llvm-mca/CodeRegion.cpp

140–141

I is passed by reference to InstrumentRegion constructor. I is a local variable in this function. Doesn't it go out of scope when this function ends, leaving the reference in InstrumentRegion dangling?

[craig.topper]

llvm/tools/llvm-mca/CodeRegion.cpp

168

emplace_back seems unnecessary here. push_back should be fine.

[craig.topper]

llvm/include/llvm/MCA/InstrBuilder.h

87

These probably don't need to be pointers to the unique_ptr. They can be raw pointers. The consumer doesn't need to know how their ownership works.

[michaelmaitland]

• Fix dangling InstrumentRegion
• Use push_back
• Use raw pointeres to consumers of Instrument

[michaelmaitland]

Fix unit tests.

[myhsu]

I don't know whether it is possible to force a lit test to use LSan. I spent the day trying to build llvm with LSan enabled without much luck. If anyone knows how to build llvm with LSan for the lit-tests, I am happy to give it another try locally.

What about Valgrind? The tests are small so it shouldn't take super long time.

[michaelmaitland]

Valgrind reports 1711 errors from 1711 contexts before my fix and 1710 errors from 1710 contexts after my fix. It looks like using valgrind has problems related to python code. This is the command I ran:

valgrind --leak-check=yes bin/llvm-lit -v ../llvm/test/tools/llvm-mca/RISCV/different-instruments.s ../llvm/test/tools/llvm-mca/RISCV/disable-im.s ../llvm/test/tools/llvm-mca/RISCV/instrument-at-start.s ../llvm/test/tools/llvm-mca/RISCV/instrument-in-middle.s ../llvm/test/tools/llvm-mca/RISCV/instrument-in-region.s ../llvm/test/tools/llvm-mca/RISCV/instrument-straddles-region.s ../llvm/test/tools/llvm-mca/RISCV/multiple-same-instruments.s ../llvm/test/tools/llvm-mca/RISCV/riscv-instrument-no-data-is-err.s ../llvm/test/tools/llvm-mca/RISCV/unknown-instrument-is-err.s ../llvm/test/tools/llvm-mca/RISCV/unknown-lmul-is-err.s

I also see these kinds of errors when I run valgrind --leak-check=yes bin/llvm-lit ../llvm/test/tools/llvm-mca/X86/* with and without my memory leak fix, although these test cases do not cause LSan to complain upstream.

I re-ran without the leack-check=yes option on RISCV tests with my fix, RISCV tests without my fix and on x86 before the original instrument changes were made and all three have many instances of definitely lost: n bytes in m blocks. I'm not sure how helpful valgrind is here as it looks like it may struggle with the python driver of llvm-lit.

[michaelmaitland]

In D150816#4354479, @michaelmaitland wrote:

Valgrind reports 1711 errors from 1711 contexts before my fix and 1710 errors from 1710 contexts after my fix. It looks like using valgrind has problems related to python code. This is the command I ran:

valgrind --leak-check=yes bin/llvm-lit -v ../llvm/test/tools/llvm-mca/RISCV/different-instruments.s ../llvm/test/tools/llvm-mca/RISCV/disable-im.s ../llvm/test/tools/llvm-mca/RISCV/instrument-at-start.s ../llvm/test/tools/llvm-mca/RISCV/instrument-in-middle.s ../llvm/test/tools/llvm-mca/RISCV/instrument-in-region.s ../llvm/test/tools/llvm-mca/RISCV/instrument-straddles-region.s ../llvm/test/tools/llvm-mca/RISCV/multiple-same-instruments.s ../llvm/test/tools/llvm-mca/RISCV/riscv-instrument-no-data-is-err.s ../llvm/test/tools/llvm-mca/RISCV/unknown-instrument-is-err.s ../llvm/test/tools/llvm-mca/RISCV/unknown-lmul-is-err.s

I also see these kinds of errors when I run valgrind --leak-check=yes bin/llvm-lit ../llvm/test/tools/llvm-mca/X86/* with and without my memory leak fix, although these test cases do not cause LSan to complain upstream.

I re-ran without the leack-check=yes option on RISCV tests with my fix, RISCV tests without my fix and on x86 before the original instrument changes were made and all three have many instances of definitely lost: n bytes in m blocks. I'm not sure how helpful valgrind is here as it looks like it may struggle with the python driver of llvm-lit.

I suspected that llvm-lit was the problem, so I re-ran valgrind directly on llvm-mca. I find that without my fix there is a 112 byte leak and after my change there is an 80 byte leak. Both of these leaks come from calling std::make_unique it seems:

==1378324== 80 bytes in 2 blocks are definitely lost in loss record 1 of 2
==1378324==    at 0x4C378C3: operator new(unsigned long) (vg_replace_malloc.c:422)
==1378324==    by 0x43CF71: llvm::mca::RISCVInstrumentManager::createInstrument(llvm::StringRef, llvm::StringRef) (in /home/mmaitland/repos/llvm-project/build/bin/llvm-mca)
==1378324==    by 0x4159D5: llvm::mca::InstrumentRegionCommentConsumer::HandleComment(llvm::SMLoc, llvm::StringRef) (in /home/mmaitland/repos/llvm-project/build/bin/llvm-mca)

Let me take another look at what is going wrong here.

[myhsu]

In D150816#4354479, @michaelmaitland wrote:

Valgrind reports 1711 errors from 1711 contexts before my fix and 1710 errors from 1710 contexts after my fix. It looks like using valgrind has problems related to python code. This is the command I ran:

Oh I meant using Valgrind directly on llvm-mca (for instance you can add a command substitution from llvm-mca to valgrind llvm-mca in local lit.cfg), which is kind of what you did in your latest comment, right?
I'm not sure why there is still leak though...

[michaelmaitland]

Fix memory leaks. This change now passes valgrind with no leaks.

[michaelmaitland]

Fix destructor.

[myhsu]

llvm/tools/llvm-mca/CodeRegionGenerator.h

93 ↗

(On Diff #523772)

was this default dtor causing any issue?

[myhsu]

In D150816#4356378, @michaelmaitland wrote:

Fix memory leaks. This change now passes valgrind with no leaks.

Nice! generally LGTM, I only have a minor comment on the dtor of CodeRegionGenerator.

[michaelmaitland]

llvm/tools/llvm-mca/CodeRegionGenerator.h

93 ↗

(On Diff #523772)

This dtor was not causing an issue since it is implemented and "Anchors" in the .cpp file. That is why I removed this change with an update to this patch.

[myhsu]

LGTM Cheers!
Please briefly explain the root cause of the leak in the commit message though.

llvm/include/llvm/MCA/CustomBehaviour.h

public:
   Instrument() : Instrument("", "") {}
 
   virtual ~Instrument() = default;
 
   StringRef getDesc() const { return Desc; }
   StringRef getData() const { return Data; }
 };
 
-using SharedInstrument = std::shared_ptr<Instrument>;
+using UniqueInstrument = std::unique_ptr<Instrument>;
 
 /// This class allows targets to optionally customize the logic that resolves
 /// scheduling class IDs. Targets can use information encoded in Instrument
 /// objects to make more informed scheduling decisions.
 class InstrumentManager {
 protected:
   const MCSubtargetInfo &STI;
   const MCInstrInfo &MCII;
 
 public:
   InstrumentManager(const MCSubtargetInfo &STI, const MCInstrInfo &MCII)
       : STI(STI), MCII(MCII) {}
 
   virtual ~InstrumentManager() = default;
 
   /// Returns true if llvm-mca should ignore instruments.
   virtual bool shouldIgnoreInstruments() const { return true; }
 
   // Returns true if this supports processing Instrument with
   // Instrument.Desc equal to Type
   virtual bool supportsInstrumentType(StringRef Type) const { return false; }
 
-  /// Allocate an Instrument, and return a shared pointer to it.
-  virtual SharedInstrument createInstrument(StringRef Desc, StringRef Data);
+  /// Allocate an Instrument, and return a unique pointer to it.
+  virtual UniqueInstrument createInstrument(StringRef Desc, StringRef Data);
 
   /// Given an MCInst and a vector of Instrument, a target can
   /// return a SchedClassID. This can be used by a subtarget to return a
   /// PseudoInstruction SchedClassID instead of the one that belongs to the
   /// BaseInstruction This can be useful when a BaseInstruction does not convey
   /// the correct scheduling information without additional data. By default,
   /// it returns the SchedClassID that belongs to MCI.
-  virtual unsigned
-  getSchedClassID(const MCInstrInfo &MCII, const MCInst &MCI,
-                  const SmallVector<SharedInstrument> &IVec) const;
+  virtual unsigned getSchedClassID(const MCInstrInfo &MCII, const MCInst &MCI,
+                                   const SmallVector<Instrument *> &IVec) const;
 };
 
 } // namespace mca
 } // namespace llvm
 
 #endif /* LLVM_MCA_CUSTOMBEHAVIOUR_H */

llvm/include/llvm/MCA/InstrBuilder.h

class InstrBuilder {
   bool FirstCallInst;
   bool FirstReturnInst;
 
   using InstRecycleCallback =
       llvm::function_ref<Instruction *(const InstrDesc &)>;
   InstRecycleCallback InstRecycleCB;
 
   Expected<const InstrDesc &>
-  createInstrDescImpl(const MCInst &MCI,
-                      const SmallVector<SharedInstrument> &IVec);
+  createInstrDescImpl(const MCInst &MCI, const SmallVector<Instrument *> &IVec);
   Expected<const InstrDesc &>
   getOrCreateInstrDesc(const MCInst &MCI,
-                       const SmallVector<SharedInstrument> &IVec);
+                       const SmallVector<Instrument *> &IVec);
 
   InstrBuilder(const InstrBuilder &) = delete;
   InstrBuilder &operator=(const InstrBuilder &) = delete;
 
   void populateWrites(InstrDesc &ID, const MCInst &MCI, unsigned SchedClassID);
   void populateReads(InstrDesc &ID, const MCInst &MCI, unsigned SchedClassID);
   Error verifyInstrDesc(const InstrDesc &ID, const MCInst &MCI) const;
 
void clear() {
     FirstReturnInst = true;
   }
 
   /// Set a callback which is invoked to retrieve a recycled mca::Instruction
   /// or null if there isn't any.
   void setInstRecycleCallback(InstRecycleCallback CB) { InstRecycleCB = CB; }
 
   Expected<std::unique_ptr<Instruction>>
-  createInstruction(const MCInst &MCI,
-                    const SmallVector<SharedInstrument> &IVec);
+  createInstruction(const MCInst &MCI, const SmallVector<Instrument *> &IVec);
 };
 } // namespace mca
 } // namespace llvm
 
 #endif // LLVM_MCA_INSTRBUILDER_H

llvm/lib/MCA/CustomBehaviour.cpp

 }
 
 std::vector<std::unique_ptr<View>>
 CustomBehaviour::getEndViews(llvm::MCInstPrinter &IP,
                              llvm::ArrayRef<llvm::MCInst> Insts) {
   return std::vector<std::unique_ptr<View>>();
 }
 
-SharedInstrument InstrumentManager::createInstrument(llvm::StringRef Desc,
+UniqueInstrument InstrumentManager::createInstrument(llvm::StringRef Desc,
                                                      llvm::StringRef Data) {
-  return std::make_shared<Instrument>(Desc, Data);
+  return std::make_unique<Instrument>(Desc, Data);
 }
 
 unsigned InstrumentManager::getSchedClassID(
     const MCInstrInfo &MCII, const MCInst &MCI,
-    const llvm::SmallVector<SharedInstrument> &IVec) const {
+    const llvm::SmallVector<Instrument *> &IVec) const {
   return MCII.get(MCI.getOpcode()).getSchedClass();
 }
 
 } // namespace mca
 } // namespace llvm

llvm/lib/MCA/InstrBuilder.cpp

Error InstrBuilder::verifyInstrDesc(const InstrDesc &ID,
   // in CodeGenSchedule.cpp.
   StringRef Message = "found an inconsistent instruction that decodes to zero "
                       "opcodes and that consumes scheduler resources.";
   return make_error<InstructionError<MCInst>>(std::string(Message), MCI);
 }
 
 Expected<const InstrDesc &>
 InstrBuilder::createInstrDescImpl(const MCInst &MCI,
-                                  const SmallVector<SharedInstrument> &IVec) {
+                                  const SmallVector<Instrument *> &IVec) {
   assert(STI.getSchedModel().hasInstrSchedModel() &&
          "Itineraries are not yet supported!");
 
   // Obtain the instruction descriptor from the opcode.
   unsigned short Opcode = MCI.getOpcode();
   const MCInstrDesc &MCDesc = MCII.get(Opcode);
   const MCSchedModel &SM = STI.getSchedModel();
 
InstrBuilder::createInstrDescImpl(const MCInst &MCI,
 
   auto VDKey = std::make_pair(&MCI, SchedClassID);
   VariantDescriptors[VDKey] = std::move(ID);
   return *VariantDescriptors[VDKey];
 }
 
 Expected<const InstrDesc &>
 InstrBuilder::getOrCreateInstrDesc(const MCInst &MCI,
-                                   const SmallVector<SharedInstrument> &IVec) {
+                                   const SmallVector<Instrument *> &IVec) {
   // Cache lookup using SchedClassID from Instrumentation
   unsigned SchedClassID = IM.getSchedClassID(MCII, MCI, IVec);
 
   auto DKey = std::make_pair(MCI.getOpcode(), SchedClassID);
   if (Descriptors.find_as(DKey) != Descriptors.end())
     return *Descriptors[DKey];
 
   unsigned CPUID = STI.getSchedModel().getProcessorID();
   SchedClassID = STI.resolveVariantSchedClass(SchedClassID, &MCI, &MCII, CPUID);
   auto VDKey = std::make_pair(&MCI, SchedClassID);
   if (VariantDescriptors.contains(VDKey))
     return *VariantDescriptors[VDKey];
 
   return createInstrDescImpl(MCI, IVec);
 }
 
 STATISTIC(NumVariantInst, "Number of MCInsts that doesn't have static Desc");
 
 Expected<std::unique_ptr<Instruction>>
 InstrBuilder::createInstruction(const MCInst &MCI,
-                                const SmallVector<SharedInstrument> &IVec) {
+                                const SmallVector<Instrument *> &IVec) {
   Expected<const InstrDesc &> DescOrErr = getOrCreateInstrDesc(MCI, IVec);
   if (!DescOrErr)
     return DescOrErr.takeError();
   const InstrDesc &D = *DescOrErr;
   Instruction *NewIS = nullptr;
   std::unique_ptr<Instruction> CreatedIS;
   bool IsInstRecycled = false;
 

llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.h

 public:
   RISCVInstrumentManager(const MCSubtargetInfo &STI, const MCInstrInfo &MCII)
       : InstrumentManager(STI, MCII) {}
 
   bool shouldIgnoreInstruments() const override { return false; }
   bool supportsInstrumentType(StringRef Type) const override;
 
   /// Create a Instrument for RISC-V target
-  SharedInstrument createInstrument(StringRef Desc, StringRef Data) override;
+  UniqueInstrument createInstrument(StringRef Desc, StringRef Data) override;
 
   /// Using the Instrument, returns a SchedClassID to use instead of
   /// the SchedClassID that belongs to the MCI or the original SchedClassID.
   unsigned
   getSchedClassID(const MCInstrInfo &MCII, const MCInst &MCI,
-                  const SmallVector<SharedInstrument> &IVec) const override;
+                  const SmallVector<Instrument *> &IVec) const override;
 };
 
 } // namespace mca
 } // namespace llvm
 
 #endif

llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp

 }
 
 bool RISCVInstrumentManager::supportsInstrumentType(
     llvm::StringRef Type) const {
   // Currently, only support for RISCVLMULInstrument type
   return Type == RISCVLMULInstrument::DESC_NAME;
 }
 
-SharedInstrument
+UniqueInstrument
 RISCVInstrumentManager::createInstrument(llvm::StringRef Desc,
                                          llvm::StringRef Data) {
   if (Desc != RISCVLMULInstrument::DESC_NAME) {
     LLVM_DEBUG(dbgs() << "RVCB: Unknown instrumentation Desc: " << Desc
                       << '\n');
     return nullptr;
   }
   if (!RISCVLMULInstrument::isDataValid(Data)) {
     LLVM_DEBUG(dbgs() << "RVCB: Bad data for instrument kind " << Desc << ": "
                       << Data << '\n');
     return nullptr;
   }
-  return std::make_shared<RISCVLMULInstrument>(Data);
+  return std::make_unique<RISCVLMULInstrument>(Data);
 }
 
 unsigned RISCVInstrumentManager::getSchedClassID(
     const MCInstrInfo &MCII, const MCInst &MCI,
-    const llvm::SmallVector<SharedInstrument> &IVec) const {
+    const llvm::SmallVector<Instrument *> &IVec) const {
   unsigned short Opcode = MCI.getOpcode();
   unsigned SchedClassID = MCII.get(Opcode).getSchedClass();
 
   for (const auto &I : IVec) {
     // Unknown Instrument kind
     if (I->getDesc() == RISCVLMULInstrument::DESC_NAME) {
-      uint8_t LMUL = static_cast<RISCVLMULInstrument *>(I.get())->getLMUL();
+      uint8_t LMUL = static_cast<RISCVLMULInstrument *>(I)->getLMUL();
       const RISCVVInversePseudosTable::PseudoInfo *RVV =
           RISCVVInversePseudosTable::getBaseInfo(Opcode, LMUL);
       // Not a RVV instr
       if (!RVV) {
         LLVM_DEBUG(
             dbgs()
             << "RVCB: Could not find PseudoInstruction for Opcode "
             << MCII.getName(Opcode) << ", LMUL=" << I->getData()

llvm/tools/llvm-mca/CodeRegion.h

class CodeRegion {
 
   CodeRegion(const CodeRegion &) = delete;
   CodeRegion &operator=(const CodeRegion &) = delete;
 
 public:
   CodeRegion(llvm::StringRef Desc, llvm::SMLoc Start)
       : Description(Desc), RangeStart(Start) {}
 
+  virtual ~CodeRegion() = default;
+
   void addInstruction(const llvm::MCInst &Instruction) {
     Instructions.emplace_back(Instruction);
   }
 
   llvm::SMLoc startLoc() const { return RangeStart; }
   llvm::SMLoc endLoc() const { return RangeEnd; }
 
   void setEndLocation(llvm::SMLoc End) { RangeEnd = End; }
   bool empty() const { return Instructions.empty(); }
   bool isLocInRange(llvm::SMLoc Loc) const;
 
   llvm::ArrayRef<llvm::MCInst> getInstructions() const { return Instructions; }
 
   llvm::StringRef getDescription() const { return Description; }
 };
 
 /// Alias AnalysisRegion with CodeRegion since CodeRegionGenerator
 /// is absract and AnalysisRegionGenerator operates on AnalysisRegions
 using AnalysisRegion = CodeRegion;
 
 /// A CodeRegion that contains instrumentation that can be used
 /// in analysis of the region.
 class InstrumentRegion : public CodeRegion {
   /// Instrument for this region.
-  SharedInstrument Instrument;
+  UniqueInstrument I;
 
 public:
-  InstrumentRegion(llvm::StringRef Desc, llvm::SMLoc Start, SharedInstrument I)
-      : CodeRegion(Desc, Start), Instrument(I) {}
+  InstrumentRegion(llvm::StringRef Desc, llvm::SMLoc Start, UniqueInstrument I)
+      : CodeRegion(Desc, Start), I(std::move(I)) {}
 
 public:
-  SharedInstrument getInstrument() const { return Instrument; }
+  Instrument *getInstrument() const { return I.get(); }
 };
 
 class CodeRegionParseError final : public Error {};
 
 class CodeRegions {
   CodeRegions(const CodeRegions &) = delete;
   CodeRegions &operator=(const CodeRegions &) = delete;
 
 protected:
   // A source manager. Used by the tool to generate meaningful warnings.
   llvm::SourceMgr &SM;
 
   using UniqueCodeRegion = std::unique_ptr<CodeRegion>;
   std::vector<UniqueCodeRegion> Regions;
   llvm::StringMap<unsigned> ActiveRegions;
   bool FoundErrors;
 
 public:
   CodeRegions(llvm::SourceMgr &S) : SM(S), FoundErrors(false) {}
+  virtual ~CodeRegions() = default;
 
   typedef std::vector<UniqueCodeRegion>::iterator iterator;
   typedef std::vector<UniqueCodeRegion>::const_iterator const_iterator;
 
   iterator begin() { return Regions.begin(); }
   iterator end() { return Regions.end(); }
   const_iterator begin() const { return Regions.cbegin(); }
   const_iterator end() const { return Regions.cend(); }

 struct AnalysisRegions : public CodeRegions {
   AnalysisRegions(llvm::SourceMgr &S);
 
   void beginRegion(llvm::StringRef Description, llvm::SMLoc Loc);
   void endRegion(llvm::StringRef Description, llvm::SMLoc Loc);
 };
 
 struct InstrumentRegions : public CodeRegions {
+
   InstrumentRegions(llvm::SourceMgr &S);
 
   void beginRegion(llvm::StringRef Description, llvm::SMLoc Loc,
-                   SharedInstrument Instrument);
+                   UniqueInstrument Instrument);
   void endRegion(llvm::StringRef Description, llvm::SMLoc Loc);
 
-  const SmallVector<SharedInstrument>
-  getActiveInstruments(llvm::SMLoc Loc) const;
+  const SmallVector<Instrument *> getActiveInstruments(llvm::SMLoc Loc) const;
 };
 
 } // namespace mca
 } // namespace llvm
 
 #endif

llvm/tools/llvm-mca/CodeRegion.cpp

if (!Description.empty()) {
     SM.PrintMessage(Loc, llvm::SourceMgr::DK_Note,
                     "unable to find an active anonymous region");
   }
 }
 
 InstrumentRegions::InstrumentRegions(llvm::SourceMgr &S) : CodeRegions(S) {}
 
 void InstrumentRegions::beginRegion(StringRef Description, SMLoc Loc,
-                                    SharedInstrument I) {
+                                    UniqueInstrument I) {
   if (Description.empty()) {
     SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error,
                     "anonymous instrumentation regions are not permitted");
     FoundErrors = true;
     return;
   }
 
   auto It = ActiveRegions.find(Description);
   if (It != ActiveRegions.end()) {
     const CodeRegion &R = *Regions[It->second];
     SM.PrintMessage(
         Loc, llvm::SourceMgr::DK_Error,
         "overlapping instrumentation regions cannot be of the same kind");
     SM.PrintMessage(R.startLoc(), llvm::SourceMgr::DK_Note,
                     "instrumentation region " + Description +
                         " was previously defined here");
     FoundErrors = true;
     return;
   }
 
   ActiveRegions[Description] = Regions.size();
-  Regions.emplace_back(std::make_unique<InstrumentRegion>(Description, Loc, I));
+  Regions.emplace_back(
+      std::make_unique<InstrumentRegion>(Description, Loc, std::move(I)));
 }
 
 void InstrumentRegions::endRegion(StringRef Description, SMLoc Loc) {
   auto It = ActiveRegions.find(Description);
   if (It != ActiveRegions.end()) {
     Regions[It->second]->setEndLocation(Loc);
     ActiveRegions.erase(It);
     return;
   }
 
   FoundErrors = true;
   SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error,
                   "found an invalid instrumentation region end directive");
   if (!Description.empty()) {
     SM.PrintMessage(Loc, llvm::SourceMgr::DK_Note,
                     "unable to find an active instrumentation region named " +
                         Description);
   }
 }
 
-const SmallVector<SharedInstrument>
+const SmallVector<Instrument *>
 InstrumentRegions::getActiveInstruments(SMLoc Loc) const {
-  SmallVector<SharedInstrument> AI;
+  SmallVector<Instrument *> AI;
   for (auto &R : Regions) {
     if (R->isLocInRange(Loc)) {
       InstrumentRegion *IR = static_cast<InstrumentRegion *>(R.get());
-      AI.emplace_back(IR->getInstrument());
+      AI.push_back(IR->getInstrument());
     }
   }
   return AI;
 }
 
 } // namespace mca
 } // namespace llvm

llvm/tools/llvm-mca/CodeRegionGenerator.cpp

if (!IM.supportsInstrumentType(InstrumentKind)) {
     else
       SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error,
                       "Unknown instrumentation type in LLVM-MCA comment: " +
                           InstrumentKind);
     FoundError = true;
     return;
   }
 
-  SharedInstrument I = IM.createInstrument(InstrumentKind, Data);
+  UniqueInstrument I = IM.createInstrument(InstrumentKind, Data);
   if (!I) {
     if (Data.empty())
       SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error,
                       "Failed to create " + InstrumentKind +
                           " instrument with no data");
     else
       SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error,
                       "Failed to create " + InstrumentKind +
                           " instrument with data: " + Data);
     FoundError = true;
     return;
   }
 
   // End InstrumentType region if one is open
   if (Regions.isRegionActive(InstrumentKind))
     Regions.endRegion(InstrumentKind, Loc);
   // Start new instrumentation region
-  Regions.beginRegion(InstrumentKind, Loc, I);
+  Regions.beginRegion(InstrumentKind, Loc, std::move(I));
 }
 
 } // namespace mca
 } // namespace llvm

llvm/tools/llvm-mca/llvm-mca.cpp

for (const std::unique_ptr<mca::AnalysisRegion> &Region : Regions) {
     ArrayRef<MCInst> Insts = Region->getInstructions();
     mca::CodeEmitter CE(*STI, *MAB, *MCE, Insts);
 
     IPP->resetState();
 
     SmallVector<std::unique_ptr<mca::Instruction>> LoweredSequence;
     for (const MCInst &MCI : Insts) {
       SMLoc Loc = MCI.getLoc();
-      const SmallVector<mca::SharedInstrument> Instruments =
+      const SmallVector<mca::Instrument *> Instruments =
           InstrumentRegions.getActiveInstruments(Loc);
 
       Expected<std::unique_ptr<mca::Instruction>> Inst =
           IB.createInstruction(MCI, Instruments);
       if (!Inst) {
         if (auto NewE = handleErrors(
                 Inst.takeError(),
                 [&IP, &STI](const mca::InstructionError<MCInst> &IE) {

llvm/unittests/tools/llvm-mca/MCATestBase.cpp

Error MCATestBase::runBaselineMCA(json::Object &Result, ArrayRef<MCInst> Insts,
                                   ArrayRef<mca::View *> Views,
                                   const mca::PipelineOptions *PO) {
   mca::Context MCA(*MRI, *STI);
 
   // Default InstrumentManager
   auto IM = std::make_unique<mca::InstrumentManager>(*STI, *MCII);
   mca::InstrBuilder IB(*STI, *MCII, *MRI, MCIA.get(), *IM);
 
-  const SmallVector<mca::SharedInstrument> Instruments;
+  const SmallVector<mca::Instrument *> Instruments;
   SmallVector<std::unique_ptr<mca::Instruction>> LoweredInsts;
   for (const auto &MCI : Insts) {
     Expected<std::unique_ptr<mca::Instruction>> Inst =
         IB.createInstruction(MCI, Instruments);
     if (!Inst) {
       if (auto NewE =
               handleErrors(Inst.takeError(),
                            [this](const mca::InstructionError<MCInst> &IE) {

llvm/unittests/tools/llvm-mca/X86/TestIncrementalMCA.cpp

TEST_F(X86TestBase, TestResumablePipeline) {
   // Add views.
   auto SV = std::make_unique<SummaryView>(STI->getSchedModel(), MCIs,
                                           PO.DispatchWidth);
   P->addEventListener(SV.get());
 
   auto IM = std::make_unique<mca::InstrumentManager>(*STI, *MCII);
   mca::InstrBuilder IB(*STI, *MCII, *MRI, MCIA.get(), *IM);
 
-  const SmallVector<mca::SharedInstrument> Instruments;
+  const SmallVector<mca::Instrument *> Instruments;
   // Tile size = 7
   for (unsigned i = 0U, E = MCIs.size(); i < E;) {
     for (unsigned TE = i + 7; i < TE && i < E; ++i) {
       Expected<std::unique_ptr<mca::Instruction>> InstOrErr =
           IB.createInstruction(MCIs[i], Instruments);
       ASSERT_TRUE(bool(InstOrErr));
       ISM.addInst(std::move(InstOrErr.get()));
     }
TEST_F(X86TestBase, TestInstructionRecycling) {
   P->addEventListener(SV.get());
 
   // Default InstrumentManager
   auto IM = std::make_unique<mca::InstrumentManager>(*STI, *MCII);
 
   mca::InstrBuilder IB(*STI, *MCII, *MRI, MCIA.get(), *IM);
   IB.setInstRecycleCallback(GetRecycledInst);
 
-  const SmallVector<mca::SharedInstrument> Instruments;
+  const SmallVector<mca::Instrument *> Instruments;
   // Tile size = 7
   for (unsigned i = 0U, E = MCIs.size(); i < E;) {
     for (unsigned TE = i + 7; i < TE && i < E; ++i) {
       Expected<std::unique_ptr<mca::Instruction>> InstOrErr =
           IB.createInstruction(MCIs[i], Instruments);
 
       if (!InstOrErr) {
         mca::Instruction *RecycledInst = nullptr;