[llvm-exegesis] Improve error reporting.

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

Reviewers: courbet

Subscribers: tschuett, llvm-commits

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

llvm-svn: 334167

tools/llvm-exegesis/lib/BenchmarkResult.cpp

@@ -210,27 +210,32 @@ unsigned BenchmarkResultContext::getInstrOpcode(llvm::StringRef Name) const {
 }
 
 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)));
-  llvm::yaml::Input Yin(*MemBuffer, getUntypedContext(Context));
-  ObjectOrList Benchmark;
-  Yin >> Benchmark;
-  return Benchmark;
+static llvm::Expected<ObjectOrList>
+readYamlCommon(const BenchmarkResultContext &Context,
+               llvm::StringRef Filename) {
+  if (auto ExpectedMemoryBuffer =
+          llvm::errorOrToExpected(llvm::MemoryBuffer::getFile(Filename))) {
+    std::unique_ptr<llvm::MemoryBuffer> MemoryBuffer =
+        std::move(ExpectedMemoryBuffer.get());
+    llvm::yaml::Input Yin(*MemoryBuffer, getUntypedContext(Context));
+    ObjectOrList Benchmark;
+    Yin >> Benchmark;
+    return Benchmark;
+  } else {
+    return ExpectedMemoryBuffer.takeError();
+  }
 }
 
-InstructionBenchmark
-InstructionBenchmark::readYamlOrDie(const BenchmarkResultContext &Context,
-                                    llvm::StringRef Filename) {
-  return readYamlOrDieCommon<InstructionBenchmark>(Context, Filename);
+llvm::Expected<InstructionBenchmark>
+InstructionBenchmark::readYaml(const BenchmarkResultContext &Context,
+                               llvm::StringRef Filename) {
+  return readYamlCommon<InstructionBenchmark>(Context, Filename);
 }
 
-std::vector<InstructionBenchmark>
-InstructionBenchmark::readYamlsOrDie(const BenchmarkResultContext &Context,
-                                     llvm::StringRef Filename) {
-  return readYamlOrDieCommon<std::vector<InstructionBenchmark>>(Context,
-                                                                Filename);
+llvm::Expected<std::vector<InstructionBenchmark>>
+InstructionBenchmark::readYamls(const BenchmarkResultContext &Context,
+                                llvm::StringRef Filename) {
+  return readYamlCommon<std::vector<InstructionBenchmark>>(Context, Filename);
 }
 
 void InstructionBenchmark::writeYamlTo(const BenchmarkResultContext &Context,
@@ -245,18 +250,21 @@ void InstructionBenchmark::readYamlFrom(const 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::writeYaml(const BenchmarkResultContext &Context,
+                                const llvm::StringRef Filename) {
   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) {

tools/llvm-exegesis/lib/BenchmarkResult.h

@@ -58,13 +58,11 @@ struct InstructionBenchmark {
   std::string Info;
 
   // Read functions.
-  static InstructionBenchmark
-  readYamlOrDie(const BenchmarkResultContext &Context,
-                llvm::StringRef Filename);
+  static llvm::Expected<InstructionBenchmark>
+  readYaml(const BenchmarkResultContext &Context, llvm::StringRef Filename);
 
-  static std::vector<InstructionBenchmark>
-  readYamlsOrDie(const BenchmarkResultContext &Context,
-                 llvm::StringRef Filename);
+  static llvm::Expected<std::vector<InstructionBenchmark>>
+  readYamls(const BenchmarkResultContext &Context, llvm::StringRef Filename);
 
   void readYamlFrom(const BenchmarkResultContext &Context,
                     llvm::StringRef InputContent);
@@ -72,8 +70,8 @@ struct InstructionBenchmark {
   // Write functions, non-const because of YAML traits.
   void writeYamlTo(const BenchmarkResultContext &Context, llvm::raw_ostream &S);
 
-  void writeYamlOrDie(const BenchmarkResultContext &Context,
-                      const llvm::StringRef Filename);
+  llvm::Error writeYaml(const BenchmarkResultContext &Context,
+                        const llvm::StringRef Filename);
 };
 
 //------------------------------------------------------------------------------

tools/llvm-exegesis/llvm-exegesis.cpp

@@ -45,7 +45,7 @@ static llvm::cl::opt<std::string>
                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(
@@ -79,6 +79,8 @@ static llvm::cl::opt<std::string>
 
 namespace exegesis {
 
+static llvm::ExitOnError ExitOnErr;
+
 static unsigned GetOpcodeOrDie(const llvm::MCInstrInfo &MCInstrInfo) {
   if (OpcodeName.empty() && (OpcodeIndex == 0))
     llvm::report_fatal_error(
@@ -138,8 +140,13 @@ void benchmarkMain() {
   if (NumRepetitions == 0)
     llvm::report_fatal_error("--num-repetitions must be greater than zero");
 
-  Runner->run(GetOpcodeOrDie(State.getInstrInfo()), Filter, NumRepetitions)
-      .writeYamlOrDie(getBenchmarkResultContext(State), BenchmarkFile);
+  // Write to standard output if file is not set.
+  if (BenchmarkFile.empty())
+    BenchmarkFile = "-";
+
+  ExitOnErr(
+      Runner->run(GetOpcodeOrDie(State.getInstrInfo()), Filter, NumRepetitions)
+          .writeYaml(getBenchmarkResultContext(State), BenchmarkFile));
   exegesis::pfm::pfmTerminate();
 }
 
@@ -157,21 +164,21 @@ static void maybeRunAnalysis(const Analysis &Analyzer, const std::string &Name,
   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::readYamls(
+          getBenchmarkResultContext(State), BenchmarkFile));
   llvm::outs() << "Parsed " << Points.size() << " benchmark points\n";
   if (Points.empty()) {
     llvm::errs() << "no benchmarks to analyze\n";

unittests/tools/llvm-exegesis/BenchmarkResultTest.cpp

@@ -55,6 +55,7 @@ static constexpr const unsigned kReg2Id = 2;
 static constexpr const char kReg2Name[] = "Reg2";
 
 TEST(BenchmarkResultTest, WriteToAndReadFromDisk) {
+  llvm::ExitOnError ExitOnErr;
   BenchmarkResultContext Ctx;
   Ctx.addInstrEntry(kInstrId, kInstrName);
   Ctx.addRegEntry(kReg1Id, kReg1Name);
@@ -83,11 +84,12 @@ TEST(BenchmarkResultTest, WriteToAndReadFromDisk) {
   EC = llvm::sys::fs::createUniqueDirectory("BenchmarkResultTestDir", Filename);
   ASSERT_FALSE(EC);
   llvm::sys::path::append(Filename, "data.yaml");
-  ToDisk.writeYamlOrDie(Ctx, Filename);
+  ExitOnErr(ToDisk.writeYaml(Ctx, Filename));
 
   {
     // One-element version.
-    const auto FromDisk = InstructionBenchmark::readYamlOrDie(Ctx, Filename);
+    const auto FromDisk =
+        ExitOnErr(InstructionBenchmark::readYaml(Ctx, Filename));
 
     EXPECT_EQ(FromDisk.Key.OpcodeName, ToDisk.Key.OpcodeName);
     EXPECT_THAT(FromDisk.Key.Instructions,
@@ -104,7 +106,7 @@ TEST(BenchmarkResultTest, WriteToAndReadFromDisk) {
   {
     // Vector version.
     const auto FromDiskVector =
-        InstructionBenchmark::readYamlsOrDie(Ctx, Filename);
+        ExitOnErr(InstructionBenchmark::readYamls(Ctx, Filename));
     ASSERT_EQ(FromDiskVector.size(), size_t{1});
     const auto FromDisk = FromDiskVector[0];
     EXPECT_EQ(FromDisk.Key.OpcodeName, ToDisk.Key.OpcodeName);