[llvm-exegesis] Improve error reporting.

[gchatelet]

Summary

BenchmarkResult IO functions now return an Error or Expected so caller can deal take proper action.

[courbet]

tools/llvm-exegesis/lib/BenchmarkResult.h

62

remove OrDie (same below)

[gchatelet]

Updating D47868: [llvm-exegesis] Improve error reporting.

tools/llvm-exegesis/lib/BenchmarkResult.h

struct InstructionBenchmark {
   std::string CpuName;
   std::string LLVMTriple;
   int NumRepetitions = 0;
   std::vector<BenchmarkMeasure> Measurements;
   std::string Error;
   std::string Info;
 
   // Read functions.
-  static InstructionBenchmark
+  static llvm::Expected<InstructionBenchmark>
   readYamlOrDie(const BenchmarkResultContext &Context,
                 llvm::StringRef Filename);
 
-  static std::vector<InstructionBenchmark>
+  static llvm::Expected<std::vector<InstructionBenchmark>>
   readYamlsOrDie(const BenchmarkResultContext &Context,
                  llvm::StringRef Filename);
 
   void readYamlFrom(const BenchmarkResultContext &Context,
                     llvm::StringRef InputContent);
 
   // Write functions, non-const because of YAML traits.
   void writeYamlTo(const BenchmarkResultContext &Context, llvm::raw_ostream &S);
 
-  void writeYamlOrDie(const BenchmarkResultContext &Context,
+  llvm::Error writeYamlOrDie(const BenchmarkResultContext &Context,
                              const llvm::StringRef Filename);
 };
 
 //------------------------------------------------------------------------------
 // Utilities to work with Benchmark measures.
 
 // A class that measures stats over benchmark measures.
 class BenchmarkMeasureStats {
 public:

tools/llvm-exegesis/lib/BenchmarkResult.cpp

 unsigned BenchmarkResultContext::getInstrOpcode(llvm::StringRef Name) const {
   const auto Itr = InstrNameToOpcode.find(Name);
   if (Itr != InstrNameToOpcode.end())
     return Itr->second;
   return 0;
 }
 
 template <typename ObjectOrList>
-static ObjectOrList readYamlOrDieCommon(const BenchmarkResultContext &Context,
-                                        llvm::StringRef Filename) {
-  std::unique_ptr<llvm::MemoryBuffer> MemBuffer = llvm::cantFail(
-      llvm::errorOrToExpected(llvm::MemoryBuffer::getFile(Filename)));
+static llvm::Expected<ObjectOrList>
+readYamlOrDieCommon(const BenchmarkResultContext &Context,
+  if (auto ExpectedMemoryBuffer =
+          llvm::errorOrToExpected(llvm::MemoryBuffer::getFile(Filename))) {
+    std::unique_ptr<llvm::MemoryBuffer> MemoryBuffer =
+        std::move(ExpectedMemoryBuffer.get());
     // YAML IO requires a mutable pointer to Context but we guarantee to not
     // modify it.
-  llvm::yaml::Input Yin(*MemBuffer,
+    llvm::yaml::Input Yin(*MemoryBuffer,
                           const_cast<BenchmarkResultContext *>(&Context));
     ObjectOrList Benchmark;
     Yin >> Benchmark;
     return Benchmark;
+  } else {
+    return ExpectedMemoryBuffer.takeError();
+  }
 }
 
-InstructionBenchmark
+llvm::Expected<InstructionBenchmark>
 InstructionBenchmark::readYamlOrDie(const BenchmarkResultContext &Context,
                                     llvm::StringRef Filename) {
   return readYamlOrDieCommon<InstructionBenchmark>(Context, Filename);
 }
 
-std::vector<InstructionBenchmark>
+llvm::Expected<std::vector<InstructionBenchmark>>
 InstructionBenchmark::readYamlsOrDie(const BenchmarkResultContext &Context,
                                      llvm::StringRef Filename) {
   return readYamlOrDieCommon<std::vector<InstructionBenchmark>>(Context,
                                                                 Filename);
 }
 
 void InstructionBenchmark::writeYamlTo(const BenchmarkResultContext &Context,
                                        llvm::raw_ostream &S) {
   // YAML IO requires a mutable pointer to Context but we guarantee to not
   // modify it.
   llvm::yaml::Output Yout(S, const_cast<BenchmarkResultContext *>(&Context));
   Yout << *this;
 }
 
 void InstructionBenchmark::readYamlFrom(const BenchmarkResultContext &Context,
                                         llvm::StringRef InputContent) {
   // YAML IO requires a mutable pointer to Context but we guarantee to not
   // modify it.
   llvm::yaml::Input Yin(InputContent,
                         const_cast<BenchmarkResultContext *>(&Context));
   Yin >> *this;
 }
 
-// FIXME: Change the API to let the caller handle errors.
-void InstructionBenchmark::writeYamlOrDie(const BenchmarkResultContext &Context,
-                                          const llvm::StringRef Filename) {
+llvm::Error
+InstructionBenchmark::writeYamlOrDie(const BenchmarkResultContext &Context,
   if (Filename == "-") {
     writeYamlTo(Context, llvm::outs());
   } else {
     int ResultFD = 0;
-    llvm::cantFail(llvm::errorCodeToError(
-        openFileForWrite(Filename, ResultFD, llvm::sys::fs::F_Text)));
+    if (auto E = llvm::errorCodeToError(
+            openFileForWrite(Filename, ResultFD, llvm::sys::fs::F_Text))) {
+      return E;
+    }
     llvm::raw_fd_ostream Ostr(ResultFD, true /*shouldClose*/);
     writeYamlTo(Context, Ostr);
   }
+  return llvm::Error::success();
 }
 
 void BenchmarkMeasureStats::push(const BenchmarkMeasure &BM) {
   if (Key.empty())
     Key = BM.Key;
   assert(Key == BM.Key);
   ++NumValues;
   SumValues += BM.Value;
   MaxValue = std::max(MaxValue, BM.Value);
   MinValue = std::min(MinValue, BM.Value);
 }
 
 } // namespace exegesis

tools/llvm-exegesis/llvm-exegesis.cpp

static llvm::cl::opt<unsigned>
     OpcodeIndex("opcode-index", llvm::cl::desc("opcode to measure, by index"),
                 llvm::cl::init(0));
 
 static llvm::cl::opt<std::string>
     OpcodeName("opcode-name", llvm::cl::desc("opcode to measure, by name"),
                llvm::cl::init(""));
 
 static llvm::cl::opt<std::string>
-    BenchmarkFile("benchmarks-file", llvm::cl::desc(""), llvm::cl::init("-"));
+    BenchmarkFile("benchmarks-file", llvm::cl::desc(""), llvm::cl::init(""));
 
 enum class BenchmarkModeE { Latency, Uops, Analysis };
 static llvm::cl::opt<BenchmarkModeE> BenchmarkMode(
     "mode", llvm::cl::desc("the mode to run"),
     llvm::cl::values(
         clEnumValN(BenchmarkModeE::Latency, "latency", "Instruction Latency"),
         clEnumValN(BenchmarkModeE::Uops, "uops", "Uop Decomposition"),
         clEnumValN(BenchmarkModeE::Analysis, "analysis", "Analysis")));
static llvm::cl::opt<std::string>
     AnalysisClustersOutputFile("analysis-clusters-output-file",
                                llvm::cl::desc(""), llvm::cl::init("-"));
 static llvm::cl::opt<std::string>
     AnalysisInconsistenciesOutputFile("analysis-inconsistencies-output-file",
                                       llvm::cl::desc(""), llvm::cl::init("-"));
 
 namespace exegesis {
 
+static llvm::ExitOnError ExitOnErr;
+
 static unsigned GetOpcodeOrDie(const llvm::MCInstrInfo &MCInstrInfo) {
   if (OpcodeName.empty() && (OpcodeIndex == 0))
     llvm::report_fatal_error(
         "please provide one and only one of 'opcode-index' or 'opcode-name'");
   if (OpcodeIndex > 0)
     return OpcodeIndex;
   // Resolve opcode name -> opcode.
   for (unsigned I = 0, E = MCInstrInfo.getNumOpcodes(); I < E; ++I)
case BenchmarkModeE::Uops:
     break;
   case BenchmarkModeE::Analysis:
     llvm_unreachable("not a benchmark");
   }
 
   if (NumRepetitions == 0)
     llvm::report_fatal_error("--num-repetitions must be greater than zero");
 
+  // Write to standard output if file is not set.
+  if (BenchmarkFile.empty())
+    BenchmarkFile = "-";
+
+  ExitOnErr(
       Runner->run(GetOpcodeOrDie(State.getInstrInfo()), Filter, NumRepetitions)
-      .writeYamlOrDie(getBenchmarkResultContext(State), BenchmarkFile);
+          .writeYamlOrDie(getBenchmarkResultContext(State), BenchmarkFile));
   exegesis::pfm::pfmTerminate();
 }
 
 // Prints the results of running analysis pass `Pass` to file `OutputFilename`
 // if OutputFilename is non-empty.
 template <typename Pass>
 static void maybeRunAnalysis(const Analysis &Analyzer, const std::string &Name,
                              const std::string &OutputFilename) {
   if (OutputFilename.empty())
     return;
   if (OutputFilename != "-") {
     llvm::errs() << "Printing " << Name << " results to file '"
                  << OutputFilename << "'\n";
   }
   std::error_code ErrorCode;
   llvm::raw_fd_ostream ClustersOS(OutputFilename, ErrorCode,
                                   llvm::sys::fs::F_RW);
-  if (ErrorCode)
-    llvm::report_fatal_error("cannot open out file: " + OutputFilename);
-  if (auto Err = Analyzer.run<Pass>(ClustersOS))
-    llvm::report_fatal_error(std::move(Err));
+  ExitOnErr(llvm::errorCodeToError(ErrorCode));
+  ExitOnErr(Analyzer.run<Pass>(ClustersOS));
 }
 
 static void analysisMain() {
+  if (BenchmarkFile.empty())
+    llvm::report_fatal_error("--benchmarks-file must be set.");
+
   llvm::InitializeNativeTarget();
   llvm::InitializeNativeTargetAsmPrinter();
-
   // Read benchmarks.
   const LLVMState State;
   const std::vector<InstructionBenchmark> Points =
-      InstructionBenchmark::readYamlsOrDie(getBenchmarkResultContext(State),
-                                           BenchmarkFile);
+      ExitOnErr(InstructionBenchmark::readYamlsOrDie(
+          getBenchmarkResultContext(State), BenchmarkFile));
   llvm::outs() << "Parsed " << Points.size() << " benchmark points\n";
   if (Points.empty()) {
     llvm::errs() << "no benchmarks to analyze\n";
     return;
   }
   // FIXME: Check that all points have the same triple/cpu.
   // FIXME: Merge points from several runs (latency and uops).
 

unittests/tools/llvm-exegesis/BenchmarkResultTest.cpp

 }
 
 namespace {
 
 static constexpr const unsigned kInstrId = 5;
 static constexpr const char kInstrName[] = "Instruction5";
 
 TEST(BenchmarkResultTest, WriteToAndReadFromDisk) {
+  llvm::ExitOnError ExitOnErr;
   BenchmarkResultContext Ctx;
   Ctx.addInstrEntry(kInstrId, kInstrName);
 
   InstructionBenchmark ToDisk;
 
   ToDisk.Key.OpcodeName = "name";
   ToDisk.Key.Instructions.push_back(llvm::MCInstBuilder(kInstrId));
   ToDisk.Key.Config = "config";
   ToDisk.Mode = InstructionBenchmark::Latency;
   ToDisk.CpuName = "cpu_name";
   ToDisk.LLVMTriple = "llvm_triple";
   ToDisk.NumRepetitions = 1;
   ToDisk.Measurements.push_back(BenchmarkMeasure{"a", 1, "debug a"});
   ToDisk.Measurements.push_back(BenchmarkMeasure{"b", 2, ""});
   ToDisk.Error = "error";
   ToDisk.Info = "info";
 
   llvm::SmallString<64> Filename;
   std::error_code EC;
   EC = llvm::sys::fs::createUniqueDirectory("BenchmarkResultTestDir", Filename);
   ASSERT_FALSE(EC);
   llvm::sys::path::append(Filename, "data.yaml");
-  ToDisk.writeYamlOrDie(Ctx, Filename);
+  ExitOnErr(ToDisk.writeYamlOrDie(Ctx, Filename));
 
   {
     // One-element version.
-    const auto FromDisk = InstructionBenchmark::readYamlOrDie(Ctx, Filename);
+    const auto FromDisk =
+        ExitOnErr(InstructionBenchmark::readYamlOrDie(Ctx, Filename));
 
     EXPECT_EQ(FromDisk.Key.OpcodeName, ToDisk.Key.OpcodeName);
     EXPECT_THAT(FromDisk.Key.Instructions,
                 Pointwise(EqMCInst(), ToDisk.Key.Instructions));
     EXPECT_EQ(FromDisk.Key.Config, ToDisk.Key.Config);
     EXPECT_EQ(FromDisk.Mode, ToDisk.Mode);
     EXPECT_EQ(FromDisk.CpuName, ToDisk.CpuName);
     EXPECT_EQ(FromDisk.LLVMTriple, ToDisk.LLVMTriple);
     EXPECT_EQ(FromDisk.NumRepetitions, ToDisk.NumRepetitions);
     EXPECT_THAT(FromDisk.Measurements, ToDisk.Measurements);
     EXPECT_THAT(FromDisk.Error, ToDisk.Error);
     EXPECT_EQ(FromDisk.Info, ToDisk.Info);
   }
   {
     // Vector version.
     const auto FromDiskVector =
-        InstructionBenchmark::readYamlsOrDie(Ctx, Filename);
+        ExitOnErr(InstructionBenchmark::readYamlsOrDie(Ctx, Filename));
     ASSERT_EQ(FromDiskVector.size(), size_t{1});
     const auto FromDisk = FromDiskVector[0];
     EXPECT_EQ(FromDisk.Key.OpcodeName, ToDisk.Key.OpcodeName);
     EXPECT_THAT(FromDisk.Key.Instructions,
                 Pointwise(EqMCInst(), ToDisk.Key.Instructions));
     EXPECT_EQ(FromDisk.Key.Config, ToDisk.Key.Config);
     EXPECT_EQ(FromDisk.Mode, ToDisk.Mode);
     EXPECT_EQ(FromDisk.CpuName, ToDisk.CpuName);