[llvm-exegesis] Refactor ExecutableFunction to use a named constructor

llvm/tools/llvm-exegesis/lib/Assembler.cpp

@@ -347,38 +347,44 @@ class TrackingSectionMemoryManager : public SectionMemoryManager {
 
 } // namespace
 
-ExecutableFunction::ExecutableFunction(
+Expected<ExecutableFunction> ExecutableFunction::create(
     std::unique_ptr<LLVMTargetMachine> TM,
-    object::OwningBinary<object::ObjectFile> &&ObjectFileHolder)
-    : Context(std::make_unique<LLVMContext>()) {
+    object::OwningBinary<object::ObjectFile> &&ObjectFileHolder) {
   assert(ObjectFileHolder.getBinary() && "cannot create object file");
+  std::unique_ptr<LLVMContext> Ctx = std::make_unique<LLVMContext>();
   // Initializing the execution engine.
   // We need to use the JIT EngineKind to be able to add an object file.
   LLVMLinkInMCJIT();
   uintptr_t CodeSize = 0;
   std::string Error;
-  ExecEngine.reset(
-      EngineBuilder(createModule(Context, TM->createDataLayout()))
+  std::unique_ptr<ExecutionEngine> EE(
+      EngineBuilder(createModule(Ctx, TM->createDataLayout()))
           .setErrorStr(&Error)
           .setMCPU(TM->getTargetCPU())
           .setEngineKind(EngineKind::JIT)
           .setMCJITMemoryManager(
               std::make_unique<TrackingSectionMemoryManager>(&CodeSize))
           .create(TM.release()));
-  if (!ExecEngine)
-    report_fatal_error(Twine(Error));
+  if (!EE)
+    return make_error<StringError>(Twine(Error), inconvertibleErrorCode());
   // Adding the generated object file containing the assembled function.
   // The ExecutionEngine makes sure the object file is copied into an
   // executable page.
-  ExecEngine->addObjectFile(std::move(ObjectFileHolder));
+  EE->addObjectFile(std::move(ObjectFileHolder));
   // Fetching function bytes.
-  const uint64_t FunctionAddress = ExecEngine->getFunctionAddress(FunctionID);
+  const uint64_t FunctionAddress = EE->getFunctionAddress(FunctionID);
   assert(isAligned(kFunctionAlignment, FunctionAddress) &&
          "function is not properly aligned");
-  FunctionBytes =
+  StringRef FBytes =
       StringRef(reinterpret_cast<const char *>(FunctionAddress), CodeSize);
+  return ExecutableFunction(std::move(Ctx), std::move(EE), FBytes);
 }
 
+ExecutableFunction::ExecutableFunction(std::unique_ptr<LLVMContext> Ctx,
+                                       std::unique_ptr<ExecutionEngine> EE,
+                                       StringRef FB)
+    : FunctionBytes(FB), Context(std::move(Ctx)), ExecEngine(std::move(EE)) {}
+
 Error getBenchmarkFunctionBytes(const StringRef InputData,
                                 std::vector<uint8_t> &Bytes) {
   const auto Holder = getObjectFromBuffer(InputData);

llvm/tools/llvm-exegesis/lib/Assembler.h

@@ -106,10 +106,11 @@ object::OwningBinary<object::ObjectFile> getObjectFromFile(StringRef Filename);
 
 // Consumes an ObjectFile containing a `void foo(char*)` function and make it
 // executable.
-struct ExecutableFunction {
-  explicit ExecutableFunction(
-      std::unique_ptr<LLVMTargetMachine> TM,
-      object::OwningBinary<object::ObjectFile> &&ObjectFileHolder);
+class ExecutableFunction {
+public:
+  static Expected<ExecutableFunction>
+  create(std::unique_ptr<LLVMTargetMachine> TM,
+         object::OwningBinary<object::ObjectFile> &&ObjectFileHolder);
 
   // Retrieves the function as an array of bytes.
   StringRef getFunctionBytes() const { return FunctionBytes; }
@@ -119,9 +120,15 @@ struct ExecutableFunction {
     ((void (*)(char *))(intptr_t)FunctionBytes.data())(Memory);
   }
 
+  StringRef FunctionBytes;
+
+private:
+  ExecutableFunction(std::unique_ptr<LLVMContext> Ctx,
+                     std::unique_ptr<ExecutionEngine> EE,
+                     StringRef FunctionBytes);
+
   std::unique_ptr<LLVMContext> Context;
   std::unique_ptr<ExecutionEngine> ExecEngine;
-  StringRef FunctionBytes;
 };
 
 // Copies benchmark function's bytes from benchmark object.

llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp

@@ -89,13 +89,25 @@ BenchmarkRunner::FunctionExecutor::runAndSample(const char *Counters) const {
 namespace {
 class InProcessFunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor {
 public:
+  static Expected<std::unique_ptr<InProcessFunctionExecutorImpl>>
+  create(const LLVMState &State, object::OwningBinary<object::ObjectFile> Obj,
+         BenchmarkRunner::ScratchSpace *Scratch) {
+    Expected<ExecutableFunction> EF =
+        ExecutableFunction::create(State.createTargetMachine(), std::move(Obj));
+
+    if (!EF)
+      return EF.takeError();
+
+    return std::unique_ptr<InProcessFunctionExecutorImpl>(
+        new InProcessFunctionExecutorImpl(State, std::move(*EF), Scratch));
+  }
+
+private:
   InProcessFunctionExecutorImpl(const LLVMState &State,
-                                object::OwningBinary<object::ObjectFile> Obj,
+                                ExecutableFunction Function,
                                 BenchmarkRunner::ScratchSpace *Scratch)
-      : State(State), Function(State.createTargetMachine(), std::move(Obj)),
-        Scratch(Scratch) {}
+      : State(State), Function(std::move(Function)), Scratch(Scratch) {}
 
-private:
   static void
   accumulateCounterValues(const llvm::SmallVector<int64_t, 4> &NewValues,
                           llvm::SmallVector<int64_t, 4> *Result) {
@@ -161,13 +173,24 @@ class InProcessFunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor {
 class SubProcessFunctionExecutorImpl
     : public BenchmarkRunner::FunctionExecutor {
 public:
+  static Expected<std::unique_ptr<SubProcessFunctionExecutorImpl>>
+  create(const LLVMState &State, object::OwningBinary<object::ObjectFile> Obj,
+         const BenchmarkKey &Key) {
+    Expected<ExecutableFunction> EF =
+        ExecutableFunction::create(State.createTargetMachine(), std::move(Obj));
+    if (!EF)
+      return EF.takeError();
+
+    return std::unique_ptr<SubProcessFunctionExecutorImpl>(
+        new SubProcessFunctionExecutorImpl(State, std::move(*EF), Key));
+  }
+
+private:
   SubProcessFunctionExecutorImpl(const LLVMState &State,
-                                 object::OwningBinary<object::ObjectFile> Obj,
+                                 ExecutableFunction Function,
                                  const BenchmarkKey &Key)
-      : State(State), Function(State.createTargetMachine(), std::move(Obj)),
-        Key(Key) {}
+      : State(State), Function(std::move(Function)), Key(Key) {}
 
-private:
   enum ChildProcessExitCodeE {
     CounterFDReadFailed = 1,
     RSeqDisableFailed,
@@ -490,17 +513,27 @@ BenchmarkRunner::createFunctionExecutor(
     object::OwningBinary<object::ObjectFile> ObjectFile,
     const BenchmarkKey &Key) const {
   switch (ExecutionMode) {
-  case ExecutionModeE::InProcess:
-    return std::make_unique<InProcessFunctionExecutorImpl>(
+  case ExecutionModeE::InProcess: {
+    auto InProcessExecutorOrErr = InProcessFunctionExecutorImpl::create(
         State, std::move(ObjectFile), Scratch.get());
-  case ExecutionModeE::SubProcess:
+    if (!InProcessExecutorOrErr)
+      return InProcessExecutorOrErr.takeError();
+
+    return std::move(*InProcessExecutorOrErr);
+  }
+  case ExecutionModeE::SubProcess: {
 #ifdef __linux__
-    return std::make_unique<SubProcessFunctionExecutorImpl>(
+    auto SubProcessExecutorOrErr = SubProcessFunctionExecutorImpl::create(
         State, std::move(ObjectFile), Key);
+    if (!SubProcessExecutorOrErr)
+      return SubProcessExecutorOrErr.takeError();
+
+    return std::move(*SubProcessExecutorOrErr);
 #else
     return make_error<Failure>(
         "The subprocess execution mode is only supported on Linux");
 #endif
+  }
   }
   llvm_unreachable("ExecutionMode is outside expected range");
 }

llvm/unittests/tools/llvm-exegesis/Common/AssemblerUtils.h

@@ -20,6 +20,7 @@
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/TargetParser/Host.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -82,8 +83,13 @@ class MachineFunctionGeneratorBaseTest : public ::testing::Test {
     EXPECT_FALSE(assembleToStream(*ET, createTargetMachine(), /*LiveIns=*/{},
                                   RegisterInitialValues, Fill, AsmStream, Key,
                                   false));
-    return ExecutableFunction(createTargetMachine(),
-                              getObjectFromBuffer(AsmStream.str()));
+    Expected<ExecutableFunction> ExecFunc = ExecutableFunction::create(
+        createTargetMachine(), getObjectFromBuffer(AsmStream.str()));
+
+    // We can't use ASSERT_THAT_EXPECTED here as it doesn't work inside of
+    // non-void functions.
+    EXPECT_TRUE(detail::TakeExpected(ExecFunc).Success());
+    return std::move(*ExecFunc);
   }
 
   const std::string TT;