[llvm] 8a02b70 - [llvm-exegesis] Refactor ExecutableFunction to use a named constructor (#72837)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 24 02:15:39 PST 2023
Author: Aiden Grossman
Date: 2023-11-24T02:15:34-08:00
New Revision: 8a02b70324f03967a6697c782d2b2436d1e1bac2
URL: https://github.com/llvm/llvm-project/commit/8a02b70324f03967a6697c782d2b2436d1e1bac2
DIFF: https://github.com/llvm/llvm-project/commit/8a02b70324f03967a6697c782d2b2436d1e1bac2.diff
LOG: [llvm-exegesis] Refactor ExecutableFunction to use a named constructor (#72837)
This patch refactors ExecutableFunction to use a named constructor
pattern, namely adding the create function, so that errors occurring
during the creation of an ExecutableFunction can be propogated back up
rather than having to deal with them in report_fatal_error.
Added:
Modified:
llvm/tools/llvm-exegesis/lib/Assembler.cpp
llvm/tools/llvm-exegesis/lib/Assembler.h
llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
llvm/unittests/tools/llvm-exegesis/Common/AssemblerUtils.h
Removed:
################################################################################
diff --git a/llvm/tools/llvm-exegesis/lib/Assembler.cpp b/llvm/tools/llvm-exegesis/lib/Assembler.cpp
index 167fb6373377c28..9ff33258e965f7e 100644
--- a/llvm/tools/llvm-exegesis/lib/Assembler.cpp
+++ b/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);
diff --git a/llvm/tools/llvm-exegesis/lib/Assembler.h b/llvm/tools/llvm-exegesis/lib/Assembler.h
index 7c2a002967af720..5f1bf8cdfb7ad6c 100644
--- a/llvm/tools/llvm-exegesis/lib/Assembler.h
+++ b/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.
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
index 19d1867fac0e2b7..85375dec2a44c30 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
+++ b/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,
@@ -497,17 +520,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");
}
diff --git a/llvm/unittests/tools/llvm-exegesis/Common/AssemblerUtils.h b/llvm/unittests/tools/llvm-exegesis/Common/AssemblerUtils.h
index 02706ca4c64cd2c..2804a6e69e824e4 100644
--- a/llvm/unittests/tools/llvm-exegesis/Common/AssemblerUtils.h
+++ b/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;
More information about the llvm-commits
mailing list