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

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Thanks for the updated patch.

I wonder if we could use MCOperandInfo::isOptionalDef() to identify the index of an optional definition.
That being said, I leave the review of `IsThumb1OptDefInst` to you and @dmgreen as I am not very familiar with the MC lowering of Thumb1 instructions.
The rest of the patch looks good to me.



================
Comment at: llvm/lib/MCA/InstrBuilder.cpp:308
+  if (IsThumb && MCDesc.hasOptionalDef())
+    NumExplicitDefs--;
+
----------------
evgeny777 wrote:
> andreadb wrote:
> > Shouldn't we update also `TotalDefs`? Otherwise we reserve the wrong number of writes at line 312.
> > 
> I think we do a fixup in the end of populateWrites function
> ```
> ID.Writes.resize(CurrentDef);
> ```
You are right. That should be enough. Thanks for checking.


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

https://reviews.llvm.org/D91704