[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
llvm-svn: 345129
diff --git a/llvm/tools/llvm-mca/include/InstrBuilder.h b/llvm/tools/llvm-mca/include/InstrBuilder.h
index ff7fb52..9fee94b 100644
--- a/llvm/tools/llvm-mca/include/InstrBuilder.h
+++ b/llvm/tools/llvm-mca/include/InstrBuilder.h
@@ -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 @@
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 @@
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);
}
diff --git a/llvm/tools/llvm-mca/include/Support.h b/llvm/tools/llvm-mca/include/Support.h
index 91c8e1b..9371394 100644
--- a/llvm/tools/llvm-mca/include/Support.h
+++ b/llvm/tools/llvm-mca/include/Support.h
@@ -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
diff --git a/llvm/tools/llvm-mca/lib/InstrBuilder.cpp b/llvm/tools/llvm-mca/lib/InstrBuilder.cpp
index 1cb020a..55f1ebf 100644
--- a/llvm/tools/llvm-mca/lib/InstrBuilder.cpp
+++ b/llvm/tools/llvm-mca/lib/InstrBuilder.cpp
@@ -215,9 +215,8 @@
}
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 @@
// 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 @@
}
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 @@
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 @@
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.
diff --git a/llvm/tools/llvm-mca/llvm-mca.cpp b/llvm/tools/llvm-mca/llvm-mca.cpp
index 59b78ff..9ad761e 100644
--- a/llvm/tools/llvm-mca/llvm-mca.cpp
+++ b/llvm/tools/llvm-mca/llvm-mca.cpp
@@ -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 @@
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 @@
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 @@
}
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 @@
*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.