[PATCH] D94928: [llvm-mca] Add support for in-order CPUs

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Thanks for the updated patch.

I left a couple of minor comments below. Otherwise the design looks good to me.

@dmgreen @evgeny777, what is your opinion on this simulation pipeline? Do you think that this design might work well for other ARM in-order processors or there are other things that should be improved? Also, if you could try a few experiments with this patch, then that would be great.

Thanks,
-Andrea



================
Comment at: llvm/include/llvm/Target/TargetSchedule.td:265-266
   bit SingleIssue = false;
+  // Allow an instruction to retire out-of-order.
+  bit RetireOOO = false;
   SchedMachineModel SchedModel = ?;
----------------
Note that instructions can have multiple writes (typically one per each register definition). So the comment should say that a 'write' is allowed to retire out-of-order.

Also, this field is only used by llvm-mca. The value of that field is ignored if the model doesn't describe an in-order processors.




================
Comment at: llvm/utils/TableGen/SubtargetEmitter.cpp:1105
         SCDesc.EndGroup |= WriteRes->getValueAsBit("SingleIssue");
+        SCDesc.RetireOOO |= WriteRes->getValueAsBit("RetireOOO");
 
----------------
Basically, an instruction is allowed to retire out-of-order if `RetireOOO` is true for at least one of its writes.
This field is only meaningful for in-order subtargets, and is ignored for other targets.

I think that this should be better described in a comment (in TargetPredicates.td).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94928