[PATCH] D59928: [MCA] Add an experimental MicroOpQueue stage.

[Clement Courbet via Phabricator via llvm-commits]

courbet added a comment.

Only cosmetic comments, this looks good !



================
Comment at: include/llvm/MCA/Context.h:42
+  unsigned MicroOpQueueSize;
+  unsigned DecodersThroughput;
   unsigned DispatchWidth;
----------------
Maybe add a comment: `// Instructions per cycle.`



================
Comment at: include/llvm/MCA/Stages/MicroOpQueueStage.h:44
+  // doesn't fit in the buffer.
+  const unsigned BufferSize;
+
----------------
This duplicates Buffer.size(), what about removing it or making it an accessor  ?


================
Comment at: tools/llvm-mca/llvm-mca.cpp:110
+    DecoderThroughput("decoder-throughput", cl::Hidden,
+                      cl::desc("Maximum throughput from the decoders"),
+                      cl::cat(ToolOptions), cl::init(0));
----------------
This can be understood as input (instructions) or output (uops) throughput, so I think we should clarify: `Maximum throughput from the decoders (instructions per cycle)`


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

https://reviews.llvm.org/D59928