[PATCH] D91704: [llvm-mca] Fix processing thumb instruction set

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

A couple of minor nits. Otherwise the llvm-mca change looks good.
Thanks.



================
Comment at: llvm/lib/MCA/InstrBuilder.cpp:261-263
   //  3. There can only be a single optional register definition, an it is
   //     always the last operand of the sequence (excluding extra operands
   //     contributed by variadic opcodes).
----------------
This comment should be updated now.

By default, the optional register definition is still expected to be the last operand. However, it can also be one of the explicit defs.
The rest of this logic still works under the assumption that instructions can only declare a single optional register definition at most.



================
Comment at: llvm/lib/MCA/InstrBuilder.cpp:311
   unsigned CurrentDef = 0;
+  unsigned OptionalDefIdx = -1U;
   unsigned i = 0;
----------------
Instead of -1, you could just default to `MCDesc.getNumOperands() - 1`.
That way, the statement at line 381 becomes a simple assignment with no conditional expression.



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

https://reviews.llvm.org/D91704