[llvm-mca] Improved error handling and error reporting from class InstrBuilder.

[andreadb]

Summary

This patch adds a new class named InstructionError to improve the error reporting in the driver.

A new class named InstructionError has been added to Support.h in order to improve the error reporting from class InstrBuilder.
The llvm-mca driver is responsible for handling InstructionError objects, and printing them out to stderr.

The goal of this patch is to remove all the remaining error handling logic from the library code.
In particular, this allows us to:

• Simplify the logic in InstrBuilder by removing a needless dependency from MCInstrPrinter.
• Centralize all the error halding logic in a new function named 'runPipeline' (see llvm-mca.cpp).

This is also a first step towards generalizing class InstrBuilder, so that in future, we will be able to reuse its logic to also "lower" MachineInstr to mca::Instruction objects.

Let me know if okay to commit.

[mattd]

LGTM. I have a few comments inline, but the code looks nice.

tools/llvm-mca/lib/InstrBuilder.cpp

334

You can probably get away with just making these StringRefs in this case.

341

I know the previous message contents are the same, but for consistency, could you add the space on the preceeding line, instead of continuing the message with a leading space.

"found an inconsistent instruction that decodes "
"to zero opcodes and that consumes scheduler "

tools/llvm-mca/llvm-mca.cpp

331

This function can be marked as a static.
STI can be made const.

[andreadb]

Thanks for the review Matt.

tools/llvm-mca/lib/InstrBuilder.cpp

334

We cannot guarantee that the storage for that string would be accessible by the time the Error object is processed. So we need to allocate a new string. Simon suggested offline to use a std::move when initializing Message. I will do that for now.

341

Will do.

tools/llvm-mca/llvm-mca.cpp

331

Right. I will change it.

test/tools/llvm-mca/ARM/unsupported-write-variant.s

+# RUN: not llvm-mca -march=arm -mcpu=swift -all-views=false 2>&1 < %s | FileCheck %s
+
+add r3, r1, r12, lsl #2
+
+# CHECK:      error: unable to resolve scheduling class for write variant.
+# CHECK-NEXT: note: instruction:    add r3, r1, r12, lsl #2

tools/llvm-mca/include/InstrBuilder.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TOOLS_LLVM_MCA_INSTRBUILDER_H
 #define LLVM_TOOLS_LLVM_MCA_INSTRBUILDER_H
 
 #include "Instruction.h"
 #include "Support.h"
-#include "llvm/MC/MCInstPrinter.h"
 #include "llvm/MC/MCInstrAnalysis.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Support/Error.h"
 
 namespace mca {
 
 /// A builder class that knows how to construct Instruction objects.
 ///
 /// Every llvm-mca Instruction is described by an object of class InstrDesc.
 /// An InstrDesc describes which registers are read/written by the instruction,
 /// as well as the instruction latency and hardware resources consumed.
 ///
 /// This class is used by the tool to construct Instructions and instruction
 /// descriptors (i.e. InstrDesc objects).
 /// Information from the machine scheduling model is used to identify processor
 /// resources that are consumed by an instruction.
 class InstrBuilder {
   const llvm::MCSubtargetInfo &STI;
   const llvm::MCInstrInfo &MCII;
   const llvm::MCRegisterInfo &MRI;
   const llvm::MCInstrAnalysis &MCIA;
-  llvm::MCInstPrinter &MCIP;
   llvm::SmallVector<uint64_t, 8> ProcResourceMasks;
 
   llvm::DenseMap<unsigned short, std::unique_ptr<const InstrDesc>> Descriptors;
   llvm::DenseMap<const llvm::MCInst *, std::unique_ptr<const InstrDesc>>
       VariantDescriptors;
 
   llvm::Expected<const InstrDesc &>
   createInstrDescImpl(const llvm::MCInst &MCI);
   llvm::Expected<const InstrDesc &>
   getOrCreateInstrDesc(const llvm::MCInst &MCI);
 
   InstrBuilder(const InstrBuilder &) = delete;
   InstrBuilder &operator=(const InstrBuilder &) = delete;
 
   llvm::Error populateWrites(InstrDesc &ID, const llvm::MCInst &MCI,
                              unsigned SchedClassID);
   llvm::Error populateReads(InstrDesc &ID, const llvm::MCInst &MCI,
                             unsigned SchedClassID);
   llvm::Error verifyInstrDesc(const InstrDesc &ID,
                               const llvm::MCInst &MCI) const;
 
 public:
   InstrBuilder(const llvm::MCSubtargetInfo &sti, const llvm::MCInstrInfo &mcii,
                const llvm::MCRegisterInfo &mri,
-               const llvm::MCInstrAnalysis &mcia, llvm::MCInstPrinter &mcip)
-      : STI(sti), MCII(mcii), MRI(mri), MCIA(mcia), MCIP(mcip),
+               const llvm::MCInstrAnalysis &mcia)
+      : STI(sti), MCII(mcii), MRI(mri), MCIA(mcia),
         ProcResourceMasks(STI.getSchedModel().getNumProcResourceKinds()) {
     computeProcResourceMasks(STI.getSchedModel(), ProcResourceMasks);
   }
 
   // Returns an array of processor resource masks.
   // Masks are computed by function mca::computeProcResourceMasks. see
   // Support.h for a description of how masks are computed and how masks can be
   // used to solve set membership problems.

tools/llvm-mca/include/Support.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TOOLS_LLVM_MCA_SUPPORT_H
 #define LLVM_TOOLS_LLVM_MCA_SUPPORT_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/MC/MCSchedule.h"
+#include "llvm/Support/Error.h"
 
 namespace mca {
 
+template<typename T>
+class InstructionError : public llvm::ErrorInfo<InstructionError<T>> {
+public:
+  static char ID;
+  std::string Message;
+  const T &Inst;
+
+  InstructionError(std::string M, const T &MCI) : Message(M), Inst(MCI) {}
+
+  void log(llvm::raw_ostream &OS) const override {
+    OS << Message;
+  }
+
+  std::error_code convertToErrorCode() const override {
+    return llvm::inconvertibleErrorCode();
+  }
+};
+
+template<typename T> char InstructionError<T>::ID;
+
 /// This class represents the number of cycles per resource (fractions of
 /// cycles).  That quantity is managed here as a ratio, and accessed via the
 /// double cast-operator below.  The two quantities, number of cycles and
 /// number of resources, are kept separate.  This is used by the
 /// ResourcePressureView to calculate the average resource cycles
 /// per instruction/iteration.
 class ResourceCycles {
   unsigned Numerator, Denominator;

tools/llvm-mca/lib/InstrBuilder.cpp

LLVM_DEBUG({
       dbgs() << "\t\t[Def] OpIdx=" << Write.OpIndex
              << ", Latency=" << Write.Latency
              << ", WriteResourceID=" << Write.SClassOrWriteResourceID << '\n';
     });
     CurrentDef++;
   }
 
   if (CurrentDef != NumExplicitDefs) {
-    return make_error<StringError>(
-        "error: Expected more register operand definitions.",
-        inconvertibleErrorCode());
+    return make_error<InstructionError<MCInst>>(
+        "Expected more register operand definitions.", MCI);
   }
 
   CurrentDef = 0;
   for (CurrentDef = 0; CurrentDef < NumImplicitDefs; ++CurrentDef) {
     unsigned Index = NumExplicitDefs + CurrentDef;
     WriteDescriptor &Write = ID.Writes[Index];
     Write.OpIndex = ~CurrentDef;
     Write.RegisterID = MCDesc.getImplicitDefs()[CurrentDef];
LLVM_DEBUG({
              << ", WriteResourceID=" << Write.SClassOrWriteResourceID << '\n';
     });
   }
 
   if (MCDesc.hasOptionalDef()) {
     // Always assume that the optional definition is the last operand of the
     // MCInst sequence.
     const MCOperand &Op = MCI.getOperand(MCI.getNumOperands() - 1);
-    if (i == MCI.getNumOperands() || !Op.isReg())
-      return make_error<StringError>(
-          "error: expected a register operand for an optional "
-          "definition. Instruction has not be correctly analyzed.",
-          inconvertibleErrorCode());
+    if (i == MCI.getNumOperands() || !Op.isReg()) {
+      std::string Message =
+          "expected a register operand for an optional definition. Instruction "
+          "has not been correctly analyzed.";
+      return make_error<InstructionError<MCInst>>(Message, MCI);
+    }
 
     WriteDescriptor &Write = ID.Writes[TotalDefs - 1];
     Write.OpIndex = MCI.getNumOperands() - 1;
     // Assign a default latency for this write.
     Write.Latency = ID.MaxLatency;
     Write.SClassOrWriteResourceID = 0;
     Write.IsOptionalDef = true;
   }
Error InstrBuilder::populateReads(InstrDesc &ID, const MCInst &MCI,
   unsigned i = 0;
   for (; i < MCI.getNumOperands() && NumExplicitDefs; ++i) {
     const MCOperand &Op = MCI.getOperand(i);
     if (Op.isReg())
       NumExplicitDefs--;
   }
 
   if (NumExplicitDefs) {
-    return make_error<StringError>(
-        "error: Expected more register operand definitions. ",
-        inconvertibleErrorCode());
+    return make_error<InstructionError<MCInst>>(
+        "Expected more register operand definitions.", MCI);
   }
 
   unsigned NumExplicitUses = MCI.getNumOperands() - i;
   unsigned NumImplicitUses = MCDesc.getNumImplicitUses();
   if (MCDesc.hasOptionalDef()) {
     assert(NumExplicitUses);
     NumExplicitUses--;
   }
if (ID.NumMicroOps != 0)
     return ErrorSuccess();
 
   bool UsesMemory = ID.MayLoad || ID.MayStore;
   bool UsesBuffers = !ID.Buffers.empty();
   bool UsesResources = !ID.Resources.empty();
   if (!UsesMemory && !UsesBuffers && !UsesResources)
     return ErrorSuccess();
 
-  std::string ToString;
-  raw_string_ostream OS(ToString);
+  std::string Message;
   if (UsesMemory) {
-    WithColor::error() << "found an inconsistent instruction that decodes "
-                       << "into zero opcodes and that consumes load/store "
-                       << "unit resources.\n";
+    Message = "found an inconsistent instruction that decodes "
+              "into zero opcodes and that consumes load/store "
+              "unit resources.";
   } else {
-    WithColor::error() << "found an inconsistent instruction that decodes"
-                       << " to zero opcodes and that consumes scheduler "
-                       << "resources.\n";
+    Message = "found an inconsistent instruction that decodes"
+              " to zero opcodes and that consumes scheduler "
+              "resources.";
   }
 
-  MCIP.printInst(&MCI, OS, "", STI);
-  OS.flush();
-  WithColor::note() << "instruction: " << ToString << '\n';
-  return make_error<StringError>("Invalid instruction definition found",
-                                 inconvertibleErrorCode());
+  return make_error<InstructionError<MCInst>>(Message, MCI);
 }
 
 Expected<const InstrDesc &>
 InstrBuilder::createInstrDescImpl(const MCInst &MCI) {
   assert(STI.getSchedModel().hasInstrSchedModel() &&
          "Itineraries are not yet supported!");
 
   // Obtain the instruction descriptor from the opcode.
   unsigned short Opcode = MCI.getOpcode();
   const MCInstrDesc &MCDesc = MCII.get(Opcode);
   const MCSchedModel &SM = STI.getSchedModel();
 
   // Then obtain the scheduling class information from the instruction.
   unsigned SchedClassID = MCDesc.getSchedClass();
   unsigned CPUID = SM.getProcessorID();
 
   // Try to solve variant scheduling classes.
   if (SchedClassID) {
     while (SchedClassID && SM.getSchedClassDesc(SchedClassID)->isVariant())
       SchedClassID = STI.resolveVariantSchedClass(SchedClassID, &MCI, CPUID);
 
     if (!SchedClassID) {
-      return make_error<StringError>("unable to resolve this variant class.",
-                                     inconvertibleErrorCode());
+      return make_error<InstructionError<MCInst>>(
+          "unable to resolve scheduling class for write variant.", MCI);
     }
   }
 
   // Check if this instruction is supported. Otherwise, report an error.
   const MCSchedClassDesc &SCDesc = *SM.getSchedClassDesc(SchedClassID);
   if (SCDesc.NumMicroOps == MCSchedClassDesc::InvalidNumMicroOps) {
-    std::string ToString;
-    raw_string_ostream OS(ToString);
-    WithColor::error() << "found an unsupported instruction in the input"
-                       << " assembly sequence.\n";
-    MCIP.printInst(&MCI, OS, "", STI);
-    OS.flush();
-    WithColor::note() << "instruction: " << ToString << '\n';
-    return make_error<StringError>(
-        "Don't know how to analyze unsupported instructions",
-        inconvertibleErrorCode());
+    return make_error<InstructionError<MCInst>>(
+        "found an unsupported instruction in the input assembly sequence.",
+        MCI);
   }
 
   // Create a new empty descriptor.
   std::unique_ptr<InstrDesc> ID = llvm::make_unique<InstrDesc>();
   ID->NumMicroOps = SCDesc.NumMicroOps;
 
   if (MCDesc.isCall()) {
     // We don't correctly model calls.

tools/llvm-mca/llvm-mca.cpp

 #include "Views/RegisterFileStatistics.h"
 #include "Views/ResourcePressureView.h"
 #include "Views/RetireControlUnitStatistics.h"
 #include "Views/SchedulerStatistics.h"
 #include "Views/SummaryView.h"
 #include "Views/TimelineView.h"
 #include "include/Context.h"
 #include "include/Pipeline.h"
+#include "include/Support.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
const cl::opt<bool> &Default =
           ? EnableAllStats
           : EnableAllViews;
   processOptionImpl(PrintRegisterFileStats, Default);
   processOptionImpl(PrintDispatchStats, Default);
   processOptionImpl(PrintSchedulerStats, Default);
   processOptionImpl(PrintRetireStats, Default);
 }
 
+// Returns true on success.
+bool runPipeline(mca::Pipeline &P, MCInstPrinter &MCIP, MCSubtargetInfo &STI) {
+  // Handle pipeline errors here.
+  if (auto Err = P.run()) {
+    if (auto NewE = handleErrors(
+            std::move(Err),
+            [&MCIP, &STI](const mca::InstructionError<MCInst> &IE) {
+              std::string InstructionStr;
+              raw_string_ostream SS(InstructionStr);
+              WithColor::error() << IE.Message << '\n';
+              MCIP.printInst(&IE.Inst, SS, "", STI);
+              SS.flush();
+              WithColor::note() << "instruction: " << InstructionStr << '\n';
+            })) {
+      // Default case.
+      WithColor::error() << toString(std::move(NewE));
+    }
+    return false;
+  }
+
+  return true;
+}
+
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
 
   // Initialize targets and assembly parsers.
   llvm::InitializeAllTargetInfos();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
 
int main(int argc, char **argv) {
 
   const MCSchedModel &SM = STI->getSchedModel();
 
   unsigned Width = SM.IssueWidth;
   if (DispatchWidth)
     Width = DispatchWidth;
 
   // Create an instruction builder.
-  mca::InstrBuilder IB(*STI, *MCII, *MRI, *MCIA, *IP);
+  mca::InstrBuilder IB(*STI, *MCII, *MRI, *MCIA);
 
   // Create a context to control ownership of the pipeline hardware.
   mca::Context MCA(*MRI, *STI);
 
   mca::PipelineOptions PO(Width, RegisterFileSize, LoadQueueSize,
                           StoreQueueSize, AssumeNoAlias);
 
   // Number each region in the sequence.
if (PrintInstructionTables) {
 
       // Create the views for this pipeline, execute, and emit a report.
       if (PrintInstructionInfoView) {
         Printer.addView(
             llvm::make_unique<mca::InstructionInfoView>(*STI, *MCII, S, *IP));
       }
       Printer.addView(
           llvm::make_unique<mca::ResourcePressureView>(*STI, *IP, S));
-      auto Err = P->run();
-      if (Err)
-        report_fatal_error(toString(std::move(Err)));
+
+      if (!runPipeline(*P, *IP, *STI))
+        return 1;
+
       Printer.printReport(TOF->os());
       continue;
     }
 
     // Create a basic pipeline simulating an out-of-order backend.
     auto P = MCA.createDefaultPipeline(PO, IB, S);
     mca::PipelinePrinter Printer(*P);
 
if (PrintResourcePressureView)
       Printer.addView(
           llvm::make_unique<mca::ResourcePressureView>(*STI, *IP, S));
 
     if (PrintTimelineView) {
       Printer.addView(llvm::make_unique<mca::TimelineView>(
           *STI, *IP, S, TimelineMaxIterations, TimelineMaxCycles));
     }
 
-    auto Err = P->run();
-    if (Err)
-      report_fatal_error(toString(std::move(Err)));
+    if (!runPipeline(*P, *IP, *STI))
+      return 1;
+
     Printer.printReport(TOF->os());
 
     // Clear the InstrBuilder internal state in preparation for another round.
     IB.clear();
   }
 
   TOF->keep();
   return 0;
 }