[PATCH] D45259: [MC][Tablegen] Allow models to describe the retire control unit for llvm-mca.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Thanks Simon.



================
Comment at: include/llvm/MC/MCSchedule.h:176
+  // micro-ops that can be retired every cycle.
+  unsigned MaxRetirePerCycle;
   const MCRegisterFileDesc *RegisterFiles;
----------------
RKSimon wrote:
> These comments repeat a lot of what is said in TargetSchedule.td - make the comments briefer here?
I will simplify the comment.


================
Comment at: tools/llvm-mca/Dispatch.h:262
 public:
-  DispatchUnit(Backend *B, const llvm::MCSubtargetInfo &STI,
-               const llvm::MCRegisterInfo &MRI, unsigned MicroOpBufferSize,
-               unsigned RegisterFileSize, unsigned MaxDispatchWidth,
-               Scheduler *Sched)
+  DispatchUnit(Backend *B, const llvm::MCSchedModel &SM,
+               const llvm::MCRegisterInfo &MRI, unsigned RegisterFileSize,
----------------
RKSimon wrote:
> Are you confident that we won't need llvm::MCSubtargetInfo again anytime soon?
Ideally, all the information needed by the dispatch logic should be accessible through the scheduling model. I may be wrong, but I don't think that we will need it again.


https://reviews.llvm.org/D45259