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

[wolfgangp]

Summary

This is a preparatory patch for PR47227. We want to separate the calculation of display data from its display for the MCA views with the goal to then alternatively generate JSON (serialized) output.

We move the calculations to methods by the name of collectData() before displaying the data.

[lebedev.ri]

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.

llvm/tools/llvm-mca/Views/InstructionInfoView.cpp

29

These should be default member-initializers

29

This is the only place with IIVDVec, let's just inline it?

43–45

auto I : zip(IIVD, Source)
const InstructionInfoViewData& IIVDEntry = std::get<0>(I)
const MCInst &Inst = std::get<1>(I)

99

Just take MutableArrayRef.

101

auto I : zip(Source, IIVD)
const MCInst &Inst = std::get<0>(I);
InstructionInfoViewData& IIVDEntry = std::get<1>(I);

[andreadb]

Nice!

Thanks Wolfgang! I always wanted to add a structured output to llvm-mca.
Thanks for working on this.

I also agree with the idea of splitting the process in two stages: a preliminar stage to collect the data from a view, a second (and final) stage where the data collected during the first stage is properly structured and printed out.

I agree with Roman in that we should not guarantee stability of data and/or structure.
Also, changes to the output structure should always be advertised (for example, by adding a line in the release notes), so that people are always aware of it.

That being said, it is not impossible for most (if not all) default views to guarantee stability of data (but not structure). After all, default views rarely (if ever) change in practice.
However, to guarantee data stability, we need some form of versioning and ideally an "auto-upgrade" functionality too (to convert/map data from an older structure to elements of the new structure). The way how I see it is that there is no need to implement this now. We can always add more guarantees in the future if we really think it is worth it.

About the patch:
Roman has already pointed out what can be improved in the patch (foreach loops / default initializer / etc.). I have nothing else to add on it.
I like your design and the general direction. I will be happy to accept this patch once comments from Roman are addressed.

Thanks,
Andrea

[wolfgangp]

Addressed review comments:

• Using MutableArrayRef
• Using range-based for loops with zip
• Using default initializers with InstructionInfoViewData

[lebedev.ri]

llvm/tools/llvm-mca/Views/InstructionInfoView.cpp

43–45

Ok, then

auto I : enumerate(zip(IIVD, Source))
const InstructionInfoViewData &IIVDEntry = std::get<0>(I.value());
CE.getEncoding(I.index())

[wolfgangp]

In D86177#2225748, @andreadb wrote:

Nice!

Thank you Andrea!

I agree with Roman in that we should not guarantee stability of data and/or structure.
Also, changes to the output structure should always be advertised (for example, by adding a line in the release notes), so that people are always aware of it.

I must confess I am not very informed about the issues regarding stability. Doesn't data stability depend on the scheduling model? I don't see how llvm-mca could guarantee it if that changes.
Or maybe I'm just confused about this. If you or Roman could briefly outline what's implied by data stability I'd appreciate it. Regarding structure stability a migration strategy would imply that llvm-mca would have to be able to read an older version JSON and update it to a later version, no?

I'll submit the follow-on patch to get started and you can educate me further on stability.

That being said, it is not impossible for most (if not all) default views to guarantee stability of data (but not structure). After all, default views rarely (if ever) change in practice.
However, to guarantee data stability, we need some form of versioning and ideally an "auto-upgrade" functionality too (to convert/map data from an older structure to elements of the new structure). The way how I see it is that there is no need to implement this now. We can always add more guarantees in the future if we really think it is worth it.

<snip>

• wolfgang

llvm/tools/llvm-mca/Views/InstructionInfoView.cpp

29

I'm planning one more use. Would you still prefer using the explicit type?

[andreadb]

In D86177#2227126, @wolfgangp wrote:

In D86177#2225748, @andreadb wrote:

Nice!

Thank you Andrea!

I agree with Roman in that we should not guarantee stability of data and/or structure.
Also, changes to the output structure should always be advertised (for example, by adding a line in the release notes), so that people are always aware of it.

I must confess I am not very informed about the issues regarding stability. Doesn't data stability depend on the scheduling model? I don't see how llvm-mca could guarantee it if that changes.

What I was trying to say is that the structure of the json report may change over time.

Future versions of mca may decide to structure data differently. Since we don't guarantee a stable structure for the json file, this may introduce incompatibilites in downstream tools which instead expect/assume a fixed/unchanging layout.

One way to solve this issue is by adding a version string to the json file, with the idea that each version string corresponds to a different structure of the json report.
It doesn't solve the incompatibility issue, but it allow tools to quickly identify incompatible files.

My other point was about the "stability of the data" (specifically: data generated by the default llvm-mca views).
Default mca views are not likely to change in future. So we can assume that the data generated by those views will always be the same. What could change is how data is layout in the final json report; Since the layout is not fixed, data may be moved to different objects/used by different name-value pairs etc.

If data doesn't change, and the only thing that changes is how data is mapped to the json structure, then it is possible to write an automated tool (e.g. an auto-upgrade script) which maps the data from an older report to the new json report structure. This again would probably require the presence of a string version.

In conclusion: if we really want to, in future we can always introduce a version string and write a simple auto-upgrade with simple rewriting rules for a few basic views.
However, yy personal opinion is that we should not worry about this (at least, not now).

[wolfgangp]

Addressed review comment:

• using enumerate() instead of an explicit index variable

[lebedev.ri]

lg

[andreadb]

LGTM

[andreadb]

Not sure why this review wasn't automatically closed.
This was committed back in August as cf6adecd6a8718ee2737ca55e4cd938364b984cc

llvm/tools/llvm-mca/Views/InstructionInfoView.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TOOLS_LLVM_MCA_INSTRUCTIONINFOVIEW_H
 #define LLVM_TOOLS_LLVM_MCA_INSTRUCTIONINFOVIEW_H
 
 #include "Views/View.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstPrinter.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MCA/CodeEmitter.h"
 #include "llvm/Support/raw_ostream.h"
 
 #define DEBUG_TYPE "llvm-mca"
 
 namespace llvm {
 namespace mca {
 
 /// A view that prints out generic instruction information.
 class InstructionInfoView : public View {
   const llvm::MCSubtargetInfo &STI;
   const llvm::MCInstrInfo &MCII;
   CodeEmitter &CE;
   bool PrintEncodings;
   llvm::ArrayRef<llvm::MCInst> Source;
   llvm::MCInstPrinter &MCIP;
 
+  struct InstructionInfoViewData {
+    unsigned NumMicroOpcodes = 0;
+    unsigned Latency = 0;
+    Optional<double> RThroughput = 0.0;
+    bool mayLoad = false;
+    bool mayStore = false;
+    bool hasUnmodeledSideEffects = false;
+  };
+  using IIVDVec = SmallVector<InstructionInfoViewData, 16>;
+
+  /// Place the data into the array of InstructionInfoViewData IIVD.
+  void collectData(MutableArrayRef<InstructionInfoViewData> IIVD) const;
+
 public:
   InstructionInfoView(const llvm::MCSubtargetInfo &ST,
                       const llvm::MCInstrInfo &II, CodeEmitter &C,
                       bool ShouldPrintEncodings, llvm::ArrayRef<llvm::MCInst> S,
                       llvm::MCInstPrinter &IP)
       : STI(ST), MCII(II), CE(C), PrintEncodings(ShouldPrintEncodings),
         Source(S), MCIP(IP) {}
 
   void printView(llvm::raw_ostream &OS) const override;
 };
 } // namespace mca
 } // namespace llvm
 
 #endif

llvm/tools/llvm-mca/Views/InstructionInfoView.cpp

 #include "llvm/Support/FormattedStream.h"
 
 namespace llvm {
 namespace mca {
 
 void InstructionInfoView::printView(raw_ostream &OS) const {
   std::string Buffer;
   raw_string_ostream TempStream(Buffer);
-  const MCSchedModel &SM = STI.getSchedModel();
-
   std::string Instruction;
   raw_string_ostream InstrStream(Instruction);
 
+  if (!Source.size())
+    return;
+
+  IIVDVec IIVD(Source.size());
+  collectData(IIVD);
+
   TempStream << "\n\nInstruction Info:\n";
   TempStream << "[1]: #uOps\n[2]: Latency\n[3]: RThroughput\n"
              << "[4]: MayLoad\n[5]: MayStore\n[6]: HasSideEffects (U)\n";
   if (PrintEncodings) {
     TempStream << "[7]: Encoding Size\n";
     TempStream << "\n[1]    [2]    [3]    [4]    [5]    [6]    [7]    "
                << "Encodings:                    Instructions:\n";
   } else {
     TempStream << "\n[1]    [2]    [3]    [4]    [5]    [6]    Instructions:\n";
   }
 
-  for (unsigned I = 0, E = Source.size(); I < E; ++I) {
-    const MCInst &Inst = Source[I];
-    const MCInstrDesc &MCDesc = MCII.get(Inst.getOpcode());
-
-    // Obtain the scheduling class information from the instruction.
-    unsigned SchedClassID = MCDesc.getSchedClass();
-    unsigned CPUID = SM.getProcessorID();
-
-    // Try to solve variant scheduling classes.
-    while (SchedClassID && SM.getSchedClassDesc(SchedClassID)->isVariant())
-      SchedClassID = STI.resolveVariantSchedClass(SchedClassID, &Inst, CPUID);
-
-    const MCSchedClassDesc &SCDesc = *SM.getSchedClassDesc(SchedClassID);
-    unsigned NumMicroOpcodes = SCDesc.NumMicroOps;
-    unsigned Latency = MCSchedModel::computeInstrLatency(STI, SCDesc);
-    // Add extra latency due to delays in the forwarding data paths.
-    Latency += MCSchedModel::getForwardingDelayCycles(
-        STI.getReadAdvanceEntries(SCDesc));
-    Optional<double> RThroughput =
-        MCSchedModel::getReciprocalThroughput(STI, SCDesc);
+  for (auto I : enumerate(zip(IIVD, Source))) {
+    const InstructionInfoViewData &IIVDEntry = std::get<0>(I.value());
 
-    TempStream << ' ' << NumMicroOpcodes << "    ";
-    if (NumMicroOpcodes < 10)
+    TempStream << ' ' << IIVDEntry.NumMicroOpcodes << "    ";
+    if (IIVDEntry.NumMicroOpcodes < 10)
       TempStream << "  ";
-    else if (NumMicroOpcodes < 100)
+    else if (IIVDEntry.NumMicroOpcodes < 100)
       TempStream << ' ';
-    TempStream << Latency << "   ";
-    if (Latency < 10)
+    TempStream << IIVDEntry.Latency << "   ";
+    if (IIVDEntry.Latency < 10)
       TempStream << "  ";
-    else if (Latency < 100)
+    else if (IIVDEntry.Latency < 100)
       TempStream << ' ';
 
-    if (RThroughput.hasValue()) {
-      double RT = RThroughput.getValue();
+    if (IIVDEntry.RThroughput.hasValue()) {
+      double RT = IIVDEntry.RThroughput.getValue();
       TempStream << format("%.2f", RT) << ' ';
       if (RT < 10.0)
         TempStream << "  ";
       else if (RT < 100.0)
         TempStream << ' ';
     } else {
       TempStream << " -     ";
     }
-    TempStream << (MCDesc.mayLoad() ? " *     " : "       ");
-    TempStream << (MCDesc.mayStore() ? " *     " : "       ");
-    TempStream << (MCDesc.hasUnmodeledSideEffects() ? " U     " : "       ");
+    TempStream << (IIVDEntry.mayLoad ? " *     " : "       ");
+    TempStream << (IIVDEntry.mayStore ? " *     " : "       ");
+    TempStream << (IIVDEntry.hasUnmodeledSideEffects ? " U     " : "       ");
 
     if (PrintEncodings) {
-      StringRef Encoding(CE.getEncoding(I));
+      StringRef Encoding(CE.getEncoding(I.index()));
       unsigned EncodingSize = Encoding.size();
       TempStream << " " << EncodingSize
                  << (EncodingSize < 10 ? "     " : "    ");
       TempStream.flush();
       formatted_raw_ostream FOS(TempStream);
       for (unsigned i = 0, e = Encoding.size(); i != e; ++i)
         FOS << format("%02x ", (uint8_t)Encoding[i]);
       FOS.PadToColumn(30);
       FOS.flush();
     }
 
+    const MCInst &Inst = std::get<1>(I.value());
     MCIP.printInst(&Inst, 0, "", STI, InstrStream);
     InstrStream.flush();
 
     // Consume any tabs or spaces at the beginning of the string.
     StringRef Str(Instruction);
     Str = Str.ltrim();
     TempStream << Str << '\n';
     Instruction = "";
   }
 
   TempStream.flush();
   OS << Buffer;
 }
+
+void InstructionInfoView::collectData(
+    MutableArrayRef<InstructionInfoViewData> IIVD) const {
+  const MCSchedModel &SM = STI.getSchedModel();
+  for (auto I : zip(Source, IIVD)) {
+    const MCInst &Inst = std::get<0>(I);
+    InstructionInfoViewData &IIVDEntry = std::get<1>(I);
+    const MCInstrDesc &MCDesc = MCII.get(Inst.getOpcode());
+
+    // Obtain the scheduling class information from the instruction.
+    unsigned SchedClassID = MCDesc.getSchedClass();
+    unsigned CPUID = SM.getProcessorID();
+
+    // Try to solve variant scheduling classes.
+    while (SchedClassID && SM.getSchedClassDesc(SchedClassID)->isVariant())
+      SchedClassID = STI.resolveVariantSchedClass(SchedClassID, &Inst, CPUID);
+
+    const MCSchedClassDesc &SCDesc = *SM.getSchedClassDesc(SchedClassID);
+    IIVDEntry.NumMicroOpcodes = SCDesc.NumMicroOps;
+    IIVDEntry.Latency = MCSchedModel::computeInstrLatency(STI, SCDesc);
+    // Add extra latency due to delays in the forwarding data paths.
+    IIVDEntry.Latency += MCSchedModel::getForwardingDelayCycles(
+        STI.getReadAdvanceEntries(SCDesc));
+    IIVDEntry.RThroughput = MCSchedModel::getReciprocalThroughput(STI, SCDesc);
+    IIVDEntry.mayLoad = MCDesc.mayLoad();
+    IIVDEntry.mayStore = MCDesc.mayStore();
+    IIVDEntry.hasUnmodeledSideEffects = MCDesc.hasUnmodeledSideEffects();
+  }
+}
 } // namespace mca.
 } // namespace llvm

llvm/tools/llvm-mca/Views/SummaryView.h

class SummaryView : public View {
   const llvm::MCSchedModel &SM;
   llvm::ArrayRef<llvm::MCInst> Source;
   const unsigned DispatchWidth;
   unsigned LastInstructionIdx;
   unsigned TotalCycles;
   // The total number of micro opcodes contributed by a block of instructions.
   unsigned NumMicroOps;
 
+  struct DisplayValues {
+    unsigned Instructions;
+    unsigned Iterations;
+    unsigned TotalInstructions;
+    unsigned TotalCycles;
+    unsigned DispatchWidth;
+    unsigned TotalUOps;
+    double IPC;
+    double UOpsPerCycle;
+    double BlockRThroughput;
+  };
+
   // For each processor resource, this vector stores the cumulative number of
   // resource cycles consumed by the analyzed code block.
   llvm::SmallVector<unsigned, 8> ProcResourceUsage;
 
   // Each processor resource is associated with a so-called processor resource
   // mask. This vector allows to correlate processor resource IDs with processor
   // resource masks. There is exactly one element per each processor resource
   // declared by the scheduling model.
   llvm::SmallVector<uint64_t, 8> ProcResourceMasks;
 
   // Used to map resource indices to actual processor resource IDs.
   llvm::SmallVector<unsigned, 8> ResIdx2ProcResID;
 
   // Compute the reciprocal throughput for the analyzed code block.
   // The reciprocal block throughput is computed as the MAX between:
   //   - NumMicroOps / DispatchWidth
   //   - Total Resource Cycles / #Units   (for every resource consumed).
   double getBlockRThroughput() const;
 
+  /// Compute the data we want to print out in the object DV.
+  void collectData(DisplayValues &DV) const;
+
 public:
   SummaryView(const llvm::MCSchedModel &Model, llvm::ArrayRef<llvm::MCInst> S,
               unsigned Width);
 
   void onCycleEnd() override { ++TotalCycles; }
   void onEvent(const HWInstructionEvent &Event) override;
   void printView(llvm::raw_ostream &OS) const override;
 };
 
 } // namespace mca
 } // namespace llvm
 
 #endif

llvm/tools/llvm-mca/Views/SummaryView.cpp

for (const std::pair<uint64_t, ResourceUsage> &RU : Desc.Resources) {
     if (RU.second.size()) {
       unsigned ProcResID = ResIdx2ProcResID[getResourceStateIndex(RU.first)];
       ProcResourceUsage[ProcResID] += RU.second.size();
     }
   }
 }
 
 void SummaryView::printView(raw_ostream &OS) const {
-  unsigned Instructions = Source.size();
-  unsigned Iterations = (LastInstructionIdx / Instructions) + 1;
-  unsigned TotalInstructions = Instructions * Iterations;
-  unsigned TotalUOps = NumMicroOps * Iterations;
-  double IPC = (double)TotalInstructions / TotalCycles;
-  double UOpsPerCycle = (double)TotalUOps / TotalCycles;
-  double BlockRThroughput = computeBlockRThroughput(
-      SM, DispatchWidth, NumMicroOps, ProcResourceUsage);
-
   std::string Buffer;
   raw_string_ostream TempStream(Buffer);
-  TempStream << "Iterations:        " << Iterations;
-  TempStream << "\nInstructions:      " << TotalInstructions;
-  TempStream << "\nTotal Cycles:      " << TotalCycles;
-  TempStream << "\nTotal uOps:        " << TotalUOps << '\n';
-  TempStream << "\nDispatch Width:    " << DispatchWidth;
+  DisplayValues DV;
+
+  collectData(DV);
+  TempStream << "Iterations:        " << DV.Iterations;
+  TempStream << "\nInstructions:      " << DV.TotalInstructions;
+  TempStream << "\nTotal Cycles:      " << DV.TotalCycles;
+  TempStream << "\nTotal uOps:        " << DV.TotalUOps << '\n';
+  TempStream << "\nDispatch Width:    " << DV.DispatchWidth;
   TempStream << "\nuOps Per Cycle:    "
-             << format("%.2f", floor((UOpsPerCycle * 100) + 0.5) / 100);
+             << format("%.2f", floor((DV.UOpsPerCycle * 100) + 0.5) / 100);
   TempStream << "\nIPC:               "
-             << format("%.2f", floor((IPC * 100) + 0.5) / 100);
+             << format("%.2f", floor((DV.IPC * 100) + 0.5) / 100);
   TempStream << "\nBlock RThroughput: "
-             << format("%.1f", floor((BlockRThroughput * 10) + 0.5) / 10)
+             << format("%.1f", floor((DV.BlockRThroughput * 10) + 0.5) / 10)
              << '\n';
   TempStream.flush();
   OS << Buffer;
 }
 
+void SummaryView::collectData(DisplayValues &DV) const {
+  DV.Instructions = Source.size();
+  DV.Iterations = (LastInstructionIdx / DV.Instructions) + 1;
+  DV.TotalInstructions = DV.Instructions * DV.Iterations;
+  DV.TotalCycles = TotalCycles;
+  DV.DispatchWidth = DispatchWidth;
+  DV.TotalUOps = NumMicroOps * DV.Iterations;
+  DV.UOpsPerCycle = (double)DV.TotalUOps / TotalCycles;
+  DV.IPC = (double)DV.TotalInstructions / TotalCycles;
+  DV.BlockRThroughput = computeBlockRThroughput(SM, DispatchWidth, NumMicroOps,
+                                                ProcResourceUsage);
+}
 } // namespace mca.
 } // namespace llvm