[PATCH] D44839: [llvm-mca] Add flag -instruction-tables to print the theoretical resource pressure distribution for instructions (PR36874)

[Clement Courbet via Phabricator via llvm-commits]

courbet accepted this revision.
courbet added inline comments.
This revision is now accepted and ready to land.


================
Comment at: tools/llvm-mca/InstrBuilder.h:54
   const InstrDesc &getOrCreateInstrDesc(const llvm::MCInst &MCI);
+  llvm::ArrayRef<uint64_t> getProcResourceMasks() const {
+    return ProcResourceMasks;
----------------
I don't remember whether I saw a comment with the definition of the masks somewhere or if we just discussed that together. Can you add a comment here, either pointing to where the notion of mask is defined, or the definition (something like "`ProcResourceMasks[I] & J` is true if resource `I` has a subresource `J`".


================
Comment at: tools/llvm-mca/InstructionTables.cpp:60
+      // Uniformly distribute Cycles across all of the units.
+      for (unsigned I1 = 0, E1 = NumUnits; I1 < E1; ++I1) {
+        unsigned SubUnitIdx = ProcResource.SubUnitsIdxBegin[I1];
----------------
Getting rid of E1 and using NumUnits instead would make everything clearer.


================
Comment at: tools/llvm-mca/InstructionTables.h:11
+///
+/// This file implements a custom driver to generate instruction tables.
+///
----------------
Please add a comment to define what "instruction tables" are, maybe just:
"see the -instruction-tables command-line flag in docs/CommandGuide/llvm-mca.rst"


https://reviews.llvm.org/D44839