[PATCH] D44980: [MC][Tblgen] Allow the definition of processor register files in the scheduling model for llvm-mca

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added inline comments.


================
Comment at: tools/llvm-mca/Dispatch.cpp:33
+  // means: this register file has an unbounded number of physical registers.
+  addRegisterFile({}, NumRegs);
+  if (!SM.hasExtraProcessorInfo())
----------------
courbet wrote:
> ```addRegisterFile({} /*all registers*/, NumRegs);```
> 
> (for readability) ?
Yes. If you think it is not needed then I remove it.


================
Comment at: utils/TableGen/SubtargetEmitter.cpp:608
 
+void SubtargetEmitter::EmitExtraProcessorInfo(const CodeGenProcModel &ProcModel,
+                                              raw_ostream &OS) {
----------------
courbet wrote:
> I'm wondering how hard it is to emit this information into a separate file ? As you mentioned in the diff description it would avoid this kind of changes impacting main llvm binaries size/compile time
My naive idea was to still keep everything into a single file, and then (maybe as a future patch) have a flag to selectively disable the emission of the extra tables for specific cpus (or entire targets). It is still possible to strip symbols from the binary if we don't want them in the binary.
To be honest, I didn't try to look into having two separate files. I guess, that can be a follow up?


================
Comment at: utils/TableGen/SubtargetEmitter.cpp:638
+  OS << "\n // {Name, #PhysRegs, #CostEntries, IndexToCostTbl}\n";
+  OS << "static const llvm::MCRegisterFileDesc " << ProcModel.ModelName
+     << "RegisterFiles"
----------------
courbet wrote:
> All valid ids in Shed (ProcRes, SchedClass, ...) start at 1. Should we add an extra invalid entry with index 0 here for consistency ?
I will do it.



https://reviews.llvm.org/D44980