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

[Clement Courbet via Phabricator via llvm-commits]

courbet added a comment.

> Information about register files is accessible through an instance of MCExtraProcessorInfo.
>  The idea behind this design is to logically partition the processor description which is only used by external tools (like llvm-mca) from the processor information used by the llvm machine schedulers.
>  I think that this design would make easier for targets to get rid of the extra processor information if they don't want it.

I like that. We're going to have similar needs with llvm-exegesis (e.g. hardware counters corresponding to resource units).



================
Comment at: tools/llvm-mca/Dispatch.cpp:33
+  // means: this register file has an unbounded number of physical registers.
+  addRegisterFile({}, NumRegs);
+  if (!SM.hasExtraProcessorInfo())
----------------
```addRegisterFile({} /*all registers*/, NumRegs);```

(for readability) ?


================
Comment at: utils/TableGen/SubtargetEmitter.cpp:608
 
+void SubtargetEmitter::EmitExtraProcessorInfo(const CodeGenProcModel &ProcModel,
+                                              raw_ostream &OS) {
----------------
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


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


https://reviews.llvm.org/D44980