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

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu added inline comments.


================
Comment at: llvm/include/llvm/MCA/CustomBehaviour.h:164
+  /// is allocated by this function.
+  virtual SmallVector<UniqueInstrument> createInstruments(const MCInst &Inst);
+
----------------
I feel like the comment here (and maybe that of `createInstrument(StringRef,StringRef)`) can hint that there are two ways to create Instruments -- by string description + data or from MCInst


================
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)) {
----------------
andreadb wrote:
> Please remove this.
please remove this or put into LLVM_DEBUG


================
Comment at: llvm/tools/llvm-mca/CodeRegionGenerator.h:136
+    errs() << "Created " << Instruments.size() << " Instruments!!\n";
+    for (UniqueInstrument &I : IM.createInstruments(Inst)) {
+      StringRef InstrumentKind = I.get()->getDesc();
----------------
we can use `Instruments` returned earlier rather than create a new one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154526