[PATCH] D86177: [llvm-mca][NFC] Separate calculation of display data from its display in the summary and instruction info views

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 19 00:20:16 PDT 2020


lebedev.ri added a comment.

Before going with JSON, i think it will be important to establish the stability guarantees:
there shouldn't be any stability guarantees, neither on the data nor on the structure.



================
Comment at: llvm/tools/llvm-mca/Views/InstructionInfoView.cpp:29
+
+  IIVDVec IIVD(Source.size(), {0, 0, 0.0, false, false, false});
+  collectData(IIVD);
----------------
These should be default member-initializers


================
Comment at: llvm/tools/llvm-mca/Views/InstructionInfoView.cpp:29
+
+  IIVDVec IIVD(Source.size(), {0, 0, 0.0, false, false, false});
+  collectData(IIVD);
----------------
lebedev.ri wrote:
> These should be default member-initializers
This is the only place with `IIVDVec`, let's just inline it?


================
Comment at: llvm/tools/llvm-mca/Views/InstructionInfoView.cpp:43-45
+  for (auto IIVDIt = IIVD.begin(); IIVDIt != IIVD.end(); ++IIVDIt) {
+    InstructionInfoViewData IIVDEntry = *IIVDIt;
+    unsigned I = std::distance(IIVD.begin(), IIVDIt);
----------------
auto I : zip(IIVD, Source)
const InstructionInfoViewData& IIVDEntry = std::get<0>(I)
const MCInst &Inst = std::get<1>(I)


================
Comment at: llvm/tools/llvm-mca/Views/InstructionInfoView.cpp:100
+
+void InstructionInfoView::collectData(IIVDVec &IIVD) const {
+  const MCSchedModel &SM = STI.getSchedModel();
----------------
Just take `MutableArrayRef`.


================
Comment at: llvm/tools/llvm-mca/Views/InstructionInfoView.cpp:102
+  const MCSchedModel &SM = STI.getSchedModel();
+  for (unsigned I = 0, E = Source.size(); I < E; ++I) {
+    InstructionInfoViewData IIVDEntry;
----------------
auto I : zip(Source, IIVD)
const MCInst &Inst = std::get<0>(I);
InstructionInfoViewData& IIVDEntry = std::get<1>(I);



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

https://reviews.llvm.org/D86177



More information about the llvm-commits mailing list