[PATCH] D142216: [MC] Store number of implicit operands in MCInstrDesc. NFC.

Mon Jan 23 12:12:59 PST 2023

foad added inline comments.

================
Comment at: llvm/utils/TableGen/InstrInfoEmitter.cpp:911-912
     Record *Inst = II->TheDef;
-    std::vector<Record*> Uses = Inst->getValueAsListOfDefs("Uses");
+    std::vector<Record *> Uses = Inst->getValueAsListOfDefs("Uses");
+    std::vector<Record *> Defs = Inst->getValueAsListOfDefs("Defs");
+    llvm::append_range(Uses, Defs);
----------------
barannikov88 wrote:
> barannikov88 wrote:
> > foad wrote:
> > > arsenm wrote:
> > > > Does this really need to generate a copy of vector for every instruction/
> > > Did you have an alternative in mind? Change `getValueAsListOfDefs` to append to an existing vector?
> > > 
> > > Or the `EmittedLists` map key could be a pair of vectors instead of a vector - but I have no idea what that would do to the overall efficiency of this code.
> > > 
> > > Would it help to //move// from Defs instead of //copy// from it?
> > `ArrayRef Uses = II->ImplicitUses;` should work.
> > 
> `PrintDefList` accepts `const std::vector<Record*> &`; that should be changed to `ArrayRef` too (otherwise the vector will be implicitly constructed).
> Unfortunately, `ArrayRef::operator std::vector<T>()` is not marked as `explicit` to catch this.
> 
I've changed this code to use `II->ImplicitUses` and `II->ImplicitDefs` (thanks!) in rG2eef8759152c7aba837addd8e90fda73c9c1f63e and rebased.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142216