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

Andrea Di Biagio via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 24 03:56:47 PDT 2018


Author: adibiagio
Date: Wed Oct 24 03:56:47 2018
New Revision: 345129

URL: http://llvm.org/viewvc/llvm-project?rev=345129&view=rev
Log:
[llvm-mca] [llvm-mca] Improved error handling and error reporting from class InstrBuilder.

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.

Differential Revision: https://reviews.llvm.org/D53585

Added:
    llvm/trunk/test/tools/llvm-mca/ARM/unsupported-write-variant.s
Modified:
    llvm/trunk/tools/llvm-mca/include/InstrBuilder.h
    llvm/trunk/tools/llvm-mca/include/Support.h
    llvm/trunk/tools/llvm-mca/lib/InstrBuilder.cpp
    llvm/trunk/tools/llvm-mca/llvm-mca.cpp

Added: llvm/trunk/test/tools/llvm-mca/ARM/unsupported-write-variant.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-mca/ARM/unsupported-write-variant.s?rev=345129&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-mca/ARM/unsupported-write-variant.s (added)
+++ llvm/trunk/test/tools/llvm-mca/ARM/unsupported-write-variant.s Wed Oct 24 03:56:47 2018
@@ -0,0 +1,6 @@
+# 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

Modified: llvm/trunk/tools/llvm-mca/include/InstrBuilder.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mca/include/InstrBuilder.h?rev=345129&r1=345128&r2=345129&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-mca/include/InstrBuilder.h (original)
+++ llvm/trunk/tools/llvm-mca/include/InstrBuilder.h Wed Oct 24 03:56:47 2018
@@ -17,7 +17,6 @@
 
 #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"
@@ -41,7 +40,6 @@ class InstrBuilder {
   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;
@@ -66,8 +64,8 @@ class InstrBuilder {
 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);
   }

Modified: llvm/trunk/tools/llvm-mca/include/Support.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mca/include/Support.h?rev=345129&r1=345128&r2=345129&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-mca/include/Support.h (original)
+++ llvm/trunk/tools/llvm-mca/include/Support.h Wed Oct 24 03:56:47 2018
@@ -18,9 +18,29 @@
 #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(std::move(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

Modified: llvm/trunk/tools/llvm-mca/lib/InstrBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mca/lib/InstrBuilder.cpp?rev=345129&r1=345128&r2=345129&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-mca/lib/InstrBuilder.cpp (original)
+++ llvm/trunk/tools/llvm-mca/lib/InstrBuilder.cpp Wed Oct 24 03:56:47 2018
@@ -215,9 +215,8 @@ Error InstrBuilder::populateWrites(Instr
   }
 
   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;
@@ -253,11 +252,12 @@ Error InstrBuilder::populateWrites(Instr
     // 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;
@@ -284,9 +284,8 @@ Error InstrBuilder::populateReads(InstrD
   }
 
   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;
@@ -332,23 +331,18 @@ Error InstrBuilder::verifyInstrDesc(cons
   if (!UsesMemory && !UsesBuffers && !UsesResources)
     return ErrorSuccess();
 
-  std::string ToString;
-  raw_string_ostream OS(ToString);
+  StringRef 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 &>
@@ -371,24 +365,17 @@ InstrBuilder::createInstrDescImpl(const
       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.

Modified: llvm/trunk/tools/llvm-mca/llvm-mca.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mca/llvm-mca.cpp?rev=345129&r1=345128&r2=345129&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-mca/llvm-mca.cpp (original)
+++ llvm/trunk/tools/llvm-mca/llvm-mca.cpp Wed Oct 24 03:56:47 2018
@@ -35,6 +35,7 @@
 #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"
@@ -326,6 +327,30 @@ static void processViewOptions() {
   processOptionImpl(PrintRetireStats, Default);
 }
 
+// Returns true on success.
+static bool runPipeline(mca::Pipeline &P, MCInstPrinter &MCIP,
+                        const 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);
 
@@ -462,7 +487,7 @@ int main(int argc, char **argv) {
     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);
@@ -504,9 +529,10 @@ int main(int argc, char **argv) {
       }
       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;
     }
@@ -543,9 +569,9 @@ int main(int argc, char **argv) {
           *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.




More information about the llvm-commits mailing list