[PATCH] D154526: [llvm-mca][RISCV] vsetivli and vsetvli act as instruments

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added inline comments.


================
Comment at: llvm/tools/llvm-mca/CodeRegionGenerator.h:121-127
+  InstrumentRegions &Regions;
+  InstrumentManager &IM;
+
+public:
+  InstrumentMCStreamer(MCContext &Context, mca::InstrumentRegions &R,
+                       InstrumentManager &IM)
+      : MCStreamerWrapper(Context, R), Regions(R), IM(IM) {}
----------------
Have you considered different approaches? Regions are already available from the base class. We don't need two references to the same data.


================
Comment at: llvm/tools/llvm-mca/CodeRegionGenerator.h:131
+                       const MCSubtargetInfo &MCSI) override {
+    // We only want to intercept the emission of new instructions.
+    MCStreamerWrapper::emitInstruction(Inst, MCSI);
----------------
You can remove this comment.


================
Comment at: llvm/tools/llvm-mca/CodeRegionGenerator.h:135
+    SmallVector<UniqueInstrument> Instruments = IM.createInstruments(Inst);
+    errs() << "Created " << Instruments.size() << " Instruments!!\n";
+    for (UniqueInstrument &I : IM.createInstruments(Inst)) {
----------------
Please remove this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154526/new/

https://reviews.llvm.org/D154526