[clang-repl] Rework layering of incremental executors.#175448
Conversation
|
@llvm/pr-subscribers-clang Author: Vassil Vassilev (vgvassilev) ChangesThe original Interpreter implementation had a hard dependency on ORC and grew organically with the addition of out-of-process JIT support. This tightly coupled the Interpreter to a specific execution engine and leaked ORC-specific assumptions (runtime layout, symbol lookup, exception model) into higher layers. The WebAssembly integration demonstrated that incremental execution can be implemented without ORC, exposing the need for a cleaner abstraction boundary. This change introduces an IncrementalExecutor interface and moves ORC-based execution behind a concrete implementation. The Interpreter now depends only on the abstract executor, improving layering and encapsulation. In addition, the Interpreter can be configured with user-provided incremental executor implementations, enabling ORC-independent execution, easier testing, and future extensions without modifying the core Interpreter. Patch is 53.76 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/175448.diff 13 Files Affected:
diff --git a/clang/include/clang/Interpreter/IncrementalExecutor.h b/clang/include/clang/Interpreter/IncrementalExecutor.h
new file mode 100644
index 0000000000000..5dfc02e46e6ce
--- /dev/null
+++ b/clang/include/clang/Interpreter/IncrementalExecutor.h
@@ -0,0 +1,95 @@
+//===--- IncrementalExecutor.h - Base Incremental Execution -----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements the base class that performs incremental code execution.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_INTERPRETER_INCREMENTALEXECUTOR_H
+#define LLVM_CLANG_LIB_INTERPRETER_INCREMENTALEXECUTOR_H
+
+#include "llvm/Support/CodeGen.h"
+#include "llvm/Support/Error.h"
+
+namespace llvm {
+namespace orc {
+class ExecutorAddr;
+class LLJITBuilder;
+class ThreadSafeContext;
+} // namespace orc
+} // namespace llvm
+
+namespace clang {
+class IncrementalExecutor;
+class TargetInfo;
+namespace driver {
+class Compilation;
+} // namespace driver
+
+// FIXME: Consider deriving from the LLJITBuilder into a common interpreter
+// creation configuraion class.
+class IncrementalExecutorBuilder {
+public:
+ /// Indicates whether out-of-process JIT execution is enabled.
+ bool IsOutOfProcess = false;
+ /// Path to the out-of-process JIT executor.
+ std::string OOPExecutor = "";
+ std::string OOPExecutorConnect = "";
+ /// Indicates whether to use shared memory for communication.
+ bool UseSharedMemory = false;
+ /// Representing the slab allocation size for memory management in kb.
+ unsigned SlabAllocateSize = 0;
+ /// Path to the ORC runtime library.
+ std::string OrcRuntimePath = "";
+ /// PID of the out-of-process JIT executor.
+ uint32_t ExecutorPID = 0;
+ /// Custom lambda to be executed inside child process/executor
+ std::function<void()> CustomizeFork = nullptr;
+ /// An optional code model to provide to the JITTargetMachineBuilder
+ std::optional<llvm::CodeModel::Model> CM = std::nullopt;
+ /// An optional external IncrementalExecutor
+ std::unique_ptr<IncrementalExecutor> IE;
+ /// An optional external orc jit builder
+ std::unique_ptr<llvm::orc::LLJITBuilder> JITBuilder;
+ /// A default callback that can be used in the IncrementalCompilerBuilder to
+ /// retrieve the path to the orc runtime.
+ std::function<llvm::Error(const driver::Compilation &)>
+ UpdateOrcRuntimePathCB = [this](const driver::Compilation &C) {
+ return UpdateOrcRuntimePath(C);
+ };
+
+ ~IncrementalExecutorBuilder();
+
+ llvm::Expected<std::unique_ptr<IncrementalExecutor>>
+ create(llvm::orc::ThreadSafeContext &TSC, const clang::TargetInfo &TI);
+
+private:
+ llvm::Error UpdateOrcRuntimePath(const driver::Compilation &C);
+};
+
+struct PartialTranslationUnit;
+
+class IncrementalExecutor {
+public:
+ enum SymbolNameKind { IRName, LinkerName };
+
+ virtual ~IncrementalExecutor() = default;
+
+ virtual llvm::Error addModule(PartialTranslationUnit &PTU) = 0;
+ virtual llvm::Error removeModule(PartialTranslationUnit &PTU) = 0;
+ virtual llvm::Error runCtors() const = 0;
+ virtual llvm::Error cleanUp() = 0;
+
+ virtual llvm::Expected<llvm::orc::ExecutorAddr>
+ getSymbolAddress(llvm::StringRef Name, SymbolNameKind NameKind) const = 0;
+ virtual llvm::Error LoadDynamicLibrary(const char *name) = 0;
+};
+
+} // namespace clang
+
+#endif // LLVM_CLANG_LIB_INTERPRETER_INCREMENTALEXECUTOR_H
diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index c4ddfb067be0f..4af5a55e6860e 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -15,6 +15,7 @@
#define LLVM_CLANG_INTERPRETER_INTERPRETER_H
#include "clang/AST/GlobalDecl.h"
+#include "clang/Interpreter/IncrementalExecutor.h"
#include "clang/Interpreter/PartialTranslationUnit.h"
#include "clang/Interpreter/Value.h"
@@ -29,8 +30,6 @@
namespace llvm {
namespace orc {
-class LLJIT;
-class LLJITBuilder;
class ThreadSafeContext;
} // namespace orc
} // namespace llvm
@@ -44,7 +43,6 @@ class Compilation;
class CompilerInstance;
class CXXRecordDecl;
class Decl;
-class IncrementalExecutor;
class IncrementalParser;
class IncrementalCUDADeviceParser;
@@ -93,42 +91,6 @@ class IncrementalCompilerBuilder {
std::optional<std::function<DriverCompilationFn>> CompilationCB;
};
-// FIXME: Consider deriving from the LLJITBuilder into a common interpreter
-// creation configuraion class.
-class IncrementalExecutorBuilder {
-public:
- /// Indicates whether out-of-process JIT execution is enabled.
- bool IsOutOfProcess = false;
- /// Path to the out-of-process JIT executor.
- std::string OOPExecutor = "";
- std::string OOPExecutorConnect = "";
- /// Indicates whether to use shared memory for communication.
- bool UseSharedMemory = false;
- /// Representing the slab allocation size for memory management in kb.
- unsigned SlabAllocateSize = 0;
- /// Path to the ORC runtime library.
- std::string OrcRuntimePath = "";
- /// PID of the out-of-process JIT executor.
- uint32_t ExecutorPID = 0;
- /// Custom lambda to be executed inside child process/executor
- std::function<void()> CustomizeFork = nullptr;
- /// An optional code model to provide to the JITTargetMachineBuilder
- std::optional<llvm::CodeModel::Model> CM = std::nullopt;
- std::function<llvm::Error(const driver::Compilation &)>
- UpdateOrcRuntimePathCB = [this](const driver::Compilation &C) {
- return UpdateOrcRuntimePath(C);
- };
-
- ~IncrementalExecutorBuilder();
-
- llvm::Expected<std::unique_ptr<IncrementalExecutor>>
- create(llvm::orc::ThreadSafeContext &TSC,
- llvm::orc::LLJITBuilder &JITBuilder);
-
-private:
- llvm::Error UpdateOrcRuntimePath(const driver::Compilation &C);
-};
-
class IncrementalAction;
class InProcessPrintingASTConsumer;
@@ -168,9 +130,8 @@ class Interpreter {
protected:
// Derived classes can use an extended interface of the Interpreter.
Interpreter(std::unique_ptr<CompilerInstance> Instance, llvm::Error &Err,
- std::unique_ptr<llvm::orc::LLJITBuilder> JITBuilder = nullptr,
- std::unique_ptr<clang::ASTConsumer> Consumer = nullptr,
- std::unique_ptr<IncrementalExecutorBuilder> IEB = nullptr);
+ std::unique_ptr<IncrementalExecutorBuilder> IEB = nullptr,
+ std::unique_ptr<clang::ASTConsumer> Consumer = nullptr);
// Create the internal IncrementalExecutor, or re-create it after calling
// ResetExecutor().
@@ -178,7 +139,7 @@ class Interpreter {
// Delete the internal IncrementalExecutor. This causes a hard shutdown of the
// JIT engine. In particular, it doesn't run cleanup or destructors.
- void ResetExecutor();
+ void ResetExecutor() { IncrExecutor.reset(); }
public:
virtual ~Interpreter();
@@ -188,15 +149,12 @@ class Interpreter {
static llvm::Expected<std::unique_ptr<Interpreter>>
createWithCUDA(std::unique_ptr<CompilerInstance> CI,
std::unique_ptr<CompilerInstance> DCI);
- static llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>>
- createLLJITBuilder(std::unique_ptr<llvm::orc::ExecutorProcessControl> EPC,
- llvm::StringRef OrcRuntimePath);
const ASTContext &getASTContext() const;
ASTContext &getASTContext();
const CompilerInstance *getCompilerInstance() const;
CompilerInstance *getCompilerInstance();
- llvm::Expected<llvm::orc::LLJIT &> getExecutionEngine();
+ llvm::Expected<IncrementalExecutor &> getExecutionEngine();
llvm::Expected<PartialTranslationUnit &> Parse(llvm::StringRef Code);
llvm::Error Execute(PartialTranslationUnit &T);
@@ -234,8 +192,6 @@ class Interpreter {
std::array<Expr *, 4> ValuePrintingInfo = {0};
- std::unique_ptr<llvm::orc::LLJITBuilder> JITBuilder;
-
std::unique_ptr<IncrementalExecutorBuilder> IncrExecutorBuilder;
/// @}
diff --git a/clang/lib/Interpreter/CMakeLists.txt b/clang/lib/Interpreter/CMakeLists.txt
index 9a597146b2fc4..01d3295d1ac30 100644
--- a/clang/lib/Interpreter/CMakeLists.txt
+++ b/clang/lib/Interpreter/CMakeLists.txt
@@ -24,6 +24,7 @@ add_clang_library(clangInterpreter
CodeCompletion.cpp
IncrementalAction.cpp
IncrementalExecutor.cpp
+ OrcIncrementalExecutor.cpp
IncrementalParser.cpp
Interpreter.cpp
InterpreterValuePrinter.cpp
diff --git a/clang/lib/Interpreter/IncrementalExecutor.cpp b/clang/lib/Interpreter/IncrementalExecutor.cpp
index f0069c4924f7a..282138e5c0ab2 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.cpp
+++ b/clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -6,56 +6,75 @@
//
//===----------------------------------------------------------------------===//
//
-// This file implements the class which performs incremental code execution.
+// This has the implementation of the base facilities for incremental execution.
//
//===----------------------------------------------------------------------===//
-#include "IncrementalExecutor.h"
+#include "clang/Interpreter/IncrementalExecutor.h"
+#include "OrcIncrementalExecutor.h"
+#ifdef __EMSCRIPTEN__
+#include "Wasm.h"
+#endif // __EMSCRIPTEN__
#include "clang/Basic/TargetInfo.h"
-#include "clang/Basic/TargetOptions.h"
-#include "clang/Interpreter/PartialTranslationUnit.h"
-#include "llvm/ADT/StringExtras.h"
-#include "llvm/ExecutionEngine/ExecutionEngine.h"
-#include "llvm/ExecutionEngine/Orc/CompileUtils.h"
+#include "clang/Driver/Compilation.h"
+#include "clang/Driver/Driver.h"
+#include "clang/Driver/ToolChain.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+
+#include "llvm/ExecutionEngine/JITLink/JITLinkMemoryManager.h"
#include "llvm/ExecutionEngine/Orc/Debugging/DebuggerSupport.h"
#include "llvm/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.h"
#include "llvm/ExecutionEngine/Orc/ExecutionUtils.h"
-#include "llvm/ExecutionEngine/Orc/IRCompileLayer.h"
+#include "llvm/ExecutionEngine/Orc/ExecutorProcessControl.h"
#include "llvm/ExecutionEngine/Orc/JITTargetMachineBuilder.h"
#include "llvm/ExecutionEngine/Orc/LLJIT.h"
#include "llvm/ExecutionEngine/Orc/MapperJITLinkMemoryManager.h"
-#include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"
#include "llvm/ExecutionEngine/Orc/Shared/OrcRTBridge.h"
#include "llvm/ExecutionEngine/Orc/Shared/SimpleRemoteEPCUtils.h"
-#include "llvm/ExecutionEngine/Orc/TargetProcess/JITLoaderGDB.h"
-#include "llvm/ExecutionEngine/SectionMemoryManager.h"
-#include "llvm/IR/Module.h"
+#include "llvm/ExecutionEngine/Orc/SimpleRemoteEPC.h"
+
+#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
-#include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/raw_ostream.h"
+
#include "llvm/TargetParser/Host.h"
+#include <array>
+#include <functional>
+#include <memory>
+#include <optional>
+#include <string>
+#include <utility>
+
#ifdef LLVM_ON_UNIX
#include <netdb.h>
#include <netinet/in.h>
#include <sys/socket.h>
#include <unistd.h>
-#endif // LLVM_ON_UNIX
-
-// Force linking some of the runtimes that helps attaching to a debugger.
-LLVM_ATTRIBUTE_USED void linkComponents() {
- llvm::errs() << (void *)&llvm_orc_registerJITLoaderGDBAllocAction;
-}
+#endif
namespace clang {
-IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC)
- : TSCtx(TSC) {}
+IncrementalExecutorBuilder::~IncrementalExecutorBuilder() = default;
-llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>>
-IncrementalExecutor::createDefaultJITBuilder(
- llvm::orc::JITTargetMachineBuilder JTMB) {
+static llvm::Expected<llvm::orc::JITTargetMachineBuilder>
+createJITTargetMachineBuilder(const llvm::Triple &TT) {
+ if (TT.getTriple() == llvm::sys::getProcessTriple())
+ // This fails immediately if the target backend is not registered
+ return llvm::orc::JITTargetMachineBuilder::detectHost();
+
+ // If the target backend is not registered, LLJITBuilder::create() will fail
+ return llvm::orc::JITTargetMachineBuilder(TT);
+}
+
+static llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>>
+createDefaultJITBuilder(llvm::orc::JITTargetMachineBuilder JTMB) {
auto JITBuilder = std::make_unique<llvm::orc::LLJITBuilder>();
JITBuilder->setJITTargetMachineBuilder(std::move(JTMB));
JITBuilder->setPrePlatformSetup([](llvm::orc::LLJIT &J) {
@@ -67,71 +86,6 @@ IncrementalExecutor::createDefaultJITBuilder(
return std::move(JITBuilder);
}
-IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC,
- llvm::orc::LLJITBuilder &JITBuilder,
- llvm::Error &Err)
- : TSCtx(TSC) {
- using namespace llvm::orc;
- llvm::ErrorAsOutParameter EAO(&Err);
-
- if (auto JitOrErr = JITBuilder.create())
- Jit = std::move(*JitOrErr);
- else {
- Err = JitOrErr.takeError();
- return;
- }
-}
-
-IncrementalExecutor::~IncrementalExecutor() {}
-
-llvm::Error IncrementalExecutor::addModule(PartialTranslationUnit &PTU) {
- llvm::orc::ResourceTrackerSP RT =
- Jit->getMainJITDylib().createResourceTracker();
- ResourceTrackers[&PTU] = RT;
-
- return Jit->addIRModule(RT, {std::move(PTU.TheModule), TSCtx});
-}
-
-llvm::Error IncrementalExecutor::removeModule(PartialTranslationUnit &PTU) {
-
- llvm::orc::ResourceTrackerSP RT = std::move(ResourceTrackers[&PTU]);
- if (!RT)
- return llvm::Error::success();
-
- ResourceTrackers.erase(&PTU);
- if (llvm::Error Err = RT->remove())
- return Err;
- return llvm::Error::success();
-}
-
-// Clean up the JIT instance.
-llvm::Error IncrementalExecutor::cleanUp() {
- // This calls the global dtors of registered modules.
- return Jit->deinitialize(Jit->getMainJITDylib());
-}
-
-llvm::Error IncrementalExecutor::runCtors() const {
- return Jit->initialize(Jit->getMainJITDylib());
-}
-
-llvm::Expected<llvm::orc::ExecutorAddr>
-IncrementalExecutor::getSymbolAddress(llvm::StringRef Name,
- SymbolNameKind NameKind) const {
- using namespace llvm::orc;
- auto SO = makeJITDylibSearchOrder({&Jit->getMainJITDylib(),
- Jit->getPlatformJITDylib().get(),
- Jit->getProcessSymbolsJITDylib().get()});
-
- ExecutionSession &ES = Jit->getExecutionSession();
-
- auto SymOrErr =
- ES.lookup(SO, (NameKind == LinkerName) ? ES.intern(Name)
- : Jit->mangleAndIntern(Name));
- if (auto Err = SymOrErr.takeError())
- return std::move(Err);
- return SymOrErr->getAddress();
-}
-
Expected<std::unique_ptr<llvm::jitlink::JITLinkMemoryManager>>
createSharedMemoryManager(llvm::orc::SimpleRemoteEPC &SREPC,
unsigned SlabAllocateSize) {
@@ -165,11 +119,10 @@ createSharedMemoryManager(llvm::orc::SimpleRemoteEPC &SREPC,
llvm::orc::SharedMemoryMapper>(SlabSize, SREPC, SAs);
}
-llvm::Expected<std::pair<std::unique_ptr<llvm::orc::SimpleRemoteEPC>, uint32_t>>
-IncrementalExecutor::launchExecutor(llvm::StringRef ExecutablePath,
- bool UseSharedMemory,
- unsigned SlabAllocateSize,
- std::function<void()> CustomizeFork) {
+static llvm::Expected<
+ std::pair<std::unique_ptr<llvm::orc::SimpleRemoteEPC>, uint32_t>>
+launchExecutor(llvm::StringRef ExecutablePath, bool UseSharedMemory,
+ unsigned SlabAllocateSize, std::function<void()> CustomizeFork) {
#ifndef LLVM_ON_UNIX
// FIXME: Add support for Windows.
return llvm::make_error<llvm::StringError>(
@@ -302,10 +255,9 @@ static Expected<int> connectTCPSocketImpl(std::string Host,
return SockFD;
}
-llvm::Expected<std::unique_ptr<llvm::orc::SimpleRemoteEPC>>
-IncrementalExecutor::connectTCPSocket(llvm::StringRef NetworkAddress,
- bool UseSharedMemory,
- unsigned SlabAllocateSize) {
+static llvm::Expected<std::unique_ptr<llvm::orc::SimpleRemoteEPC>>
+connectTCPSocket(llvm::StringRef NetworkAddress, bool UseSharedMemory,
+ unsigned SlabAllocateSize) {
#ifndef LLVM_ON_UNIX
// FIXME: Add TCP support for Windows.
return llvm::make_error<llvm::StringError>(
@@ -357,4 +309,202 @@ IncrementalExecutor::connectTCPSocket(llvm::StringRef NetworkAddress,
}
#endif // _WIN32
-} // namespace clang
+static llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>>
+createLLJITBuilder(std::unique_ptr<llvm::orc::ExecutorProcessControl> EPC,
+ llvm::StringRef OrcRuntimePath) {
+ auto JTMB = createJITTargetMachineBuilder(EPC->getTargetTriple());
+ if (!JTMB)
+ return JTMB.takeError();
+ auto JB = createDefaultJITBuilder(std::move(*JTMB));
+ if (!JB)
+ return JB.takeError();
+
+ (*JB)->setExecutorProcessControl(std::move(EPC));
+ (*JB)->setPlatformSetUp(
+ llvm::orc::ExecutorNativePlatform(OrcRuntimePath.str()));
+
+ return std::move(*JB);
+}
+
+static llvm::Expected<
+ std::pair<std::unique_ptr<llvm::orc::LLJITBuilder>, uint32_t>>
+outOfProcessJITBuilder(const IncrementalExecutorBuilder &IncrExecutorBuilder) {
+ std::unique_ptr<llvm::orc::ExecutorProcessControl> EPC;
+ uint32_t childPid = -1;
+ if (!IncrExecutorBuilder.OOPExecutor.empty()) {
+ // Launch an out-of-process executor locally in a child process.
+ auto ResultOrErr = launchExecutor(IncrExecutorBuilder.OOPExecutor,
+ IncrExecutorBuilder.UseSharedMemory,
+ IncrExecutorBuilder.SlabAllocateSize,
+ IncrExecutorBuilder.CustomizeFork);
+ if (!ResultOrErr)
+ return ResultOrErr.takeError();
+ childPid = ResultOrErr->second;
+ auto EPCOrErr = std::move(ResultOrErr->first);
+ EPC = std::move(EPCOrErr);
+ } else if (IncrExecutorBuilder.OOPExecutorConnect != "") {
+#if LLVM_ON_UNIX && LLVM_ENABLE_THREADS
+ auto EPCOrErr = connectTCPSocket(IncrExecutorBuilder.OOPExecutorConnect,
+ IncrExecutorBuilder.UseSharedMemory,
+ IncrExecutorBuilder.SlabAllocateSize);
+ if (!EPCOrErr)
+ return EPCOrErr.takeError();
+ EPC = std::move(*EPCOrErr);
+#else
+ return llvm::make_error<llvm::StringError>(
+ "Out-of-process JIT over TCP is not supported on this platform",
+ std::error_code());
+#endif
+ }
+
+ std::unique_ptr<llvm::orc::LLJITBuilder> JB;
+ if (EPC) {
+ auto JBOrErr =
+ createLLJITBuilder(std::move(EPC), IncrExecutorBuilder.OrcRuntimePath);
+ if (!JBOrErr)
+ return JBOrErr.takeError();
+ JB = std::move(*JBOrErr);
+ }
+
+ return std::make_pair(std::move(JB), childPid);
+}
+
+llvm::Expected<std::unique_ptr<IncrementalExecutor>>
+IncrementalExecutorBuilder::create(llvm::orc::ThreadSafeContext &TSC,
+ const clang::TargetInfo &TI) {
+ if (IE)
+ return std::move(IE);
+ llvm::Triple TT = TI.getTriple();
+ if (!TT.isOSWindows() && IsOutOfProcess) {
+ if (!JITBuilder) {
+ auto ResOrErr = outOfProcessJITBuilder(*this);
+ if (!ResOrErr)
+ return ResOrErr.takeError();
+ JITBuilder = std::move(ResOrErr->first);
+ ExecutorPID = ResOrErr->second;
+ }
+ if (!JITBuilder)
+ return llvm::make_error<llvm::StringError>(
+ "Operation failed. No LLJITBuilder for out-of-process JIT",
+ std::error_code());
+ }
+
+ if (!JITBuilder) {
+ auto JTMB = createJITTargetMachineBuilder(TT);
+ if (!JTMB)
+ return JTMB.takeError();
+ if (CM)
+ JTMB->setCodeModel(CM);
+ auto JB = createDefaultJITBuilder(std::move(*JTMB));
+ if (!JB)
+ return JB.takeError();
+ JITBuilder = std::move(*JB);
+ }
+
+ llvm::Error Err = llvm::Error::success();
+ std::unique_ptr<IncrementalExecutor> Executor;
+#ifdef __EMSCRIPTEN__
+ Executor = std::make_unique<WasmIncrementalExecutor>(TSC);
+#else
+ Executor = s...
[truncated]
|
|
@mcbarton, @anutosh491 can we check if the wasm changes compile and work as expected? cc: @aaronj0. |
6677179 to
4bbe72b
Compare
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
|
@vgvassilev When trying to do an Emscripten build of llvm with the branch of this PR I get the following error |
The original Interpreter implementation had a hard dependency on ORC and grew organically with the addition of out-of-process JIT support. This tightly coupled the Interpreter to a specific execution engine and leaked ORC-specific assumptions (runtime layout, symbol lookup, exception model) into higher layers. The WebAssembly integration demonstrated that incremental execution can be implemented without ORC, exposing the need for a cleaner abstraction boundary. This change introduces an IncrementalExecutor interface and moves ORC-based execution behind a concrete implementation. The Interpreter now depends only on the abstract executor, improving layering and encapsulation. In addition, the Interpreter can be configured with user-provided incremental executor implementations, enabling ORC-independent execution, easier testing, and future extensions without modifying the core Interpreter.
4bbe72b to
a5086f1
Compare
Ah, thanks! I believe I have a fix now, can you check? |
Wow, thanks a lot for this. I was just about to open an issue for this by today or tomorrow .... and start sending patches for the same. We've discussed dropping the size for the wasm build of I'll try the wasm build on this commit and get back. |
|
Can you also try this patch on top of this PR: diff --git a/clang/lib/Interpreter/IncrementalExecutor.cpp b/clang/lib/Interpreter/IncrementalExecutor.cpp
index 001651522c32..74b751d0e2da 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.cpp
+++ b/clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -404,7 +404,7 @@ IncrementalExecutorBuilder::create(llvm::orc::ThreadSafeContext &TSC,
llvm::Error Err = llvm::Error::success();
std::unique_ptr<IncrementalExecutor> Executor;
#ifdef __EMSCRIPTEN__
- Executor = std::make_unique<WasmIncrementalExecutor>();
+ Executor = std::make_unique<WasmIncrementalExecutor>(Err);
#else
Executor = std::make_unique<OrcIncrementalExecutor>(TSC, *JITBuilder, Err);
#endif
diff --git a/clang/lib/Interpreter/Wasm.cpp b/clang/lib/Interpreter/Wasm.cpp
index 56f51e5d5311..68bfb871367d 100644
--- a/clang/lib/Interpreter/Wasm.cpp
+++ b/clang/lib/Interpreter/Wasm.cpp
@@ -57,7 +57,20 @@ bool link(llvm::ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS,
namespace clang {
-WasmIncrementalExecutor::WasmIncrementalExecutor() = default;
+WasmIncrementalExecutor::WasmIncrementalExecutor(llvm::Error &Err) {
+ llvm::ErrorAsOutParameter EAO(&Err);
+
+ if (Err)
+ return;
+
+ if (auto EC =
+ llvm::sys::fs::createUniqueDirectory("clang-wasm-exec-", TempDir))
+ Err = llvm::make_error<llvm::StringError>(
+ "Failed to create temporary directory for Wasm executor: " +
+ EC.message(),
+ llvm::inconvertibleErrorCode());
+}
+
WasmIncrementalExecutor::~WasmIncrementalExecutor() = default;
llvm::Error WasmIncrementalExecutor::addModule(PartialTranslationUnit &PTU) {
@@ -74,11 +87,20 @@ llvm::Error WasmIncrementalExecutor::addModule(PartialTranslationUnit &PTU) {
llvm::TargetMachine *TargetMachine = Target->createTargetMachine(
PTU.TheModule->getTargetTriple(), "", "", TO, llvm::Reloc::Model::PIC_);
PTU.TheModule->setDataLayout(TargetMachine->createDataLayout());
- std::string ObjectFileName = PTU.TheModule->getName().str() + ".o";
- std::string BinaryFileName = PTU.TheModule->getName().str() + ".wasm";
- std::error_code Error;
- llvm::raw_fd_ostream ObjectFileOutput(llvm::StringRef(ObjectFileName), Error);
+ llvm::SmallString<256> ObjectFileName(TempDir);
+ llvm::sys::path::append(ObjectFileName,
+ PTU.TheModule->getName() + ".o");
+
+ llvm::SmallString<256> BinaryFileName(TempDir);
+ llvm::sys::path::append(BinaryFileName,
+ PTU.TheModule->getName() + ".wasm");
+
+ std::error_code EC;
+ llvm::raw_fd_ostream ObjectFileOutput(ObjectFileName, EC);
+
+ if (EC)
+ return llvm::errorCodeToError(EC);
llvm::legacy::PassManager PM;
if (TargetMachine->addPassesToEmitFile(PM, ObjectFileOutput, nullptr,It should get rid of one of the downstream patches we have. |
perfect. Having a go at it now ! |
anutosh491
left a comment
There was a problem hiding this comment.
Looks great to me
Here's what I did
- Fetched the commit and built it for wasm.
- Ran
node ./ClangReplInterpreterTests.jsand everything passes as expected
anutosh491@Anutoshs-MacBook-Air Interpreter % node ./ClangReplInterpreterTests.js
[==========] Running 27 tests from 5 test suites.
[----------] Global test environment set-up.
[----------] 2 tests from IncrementalCompilerBuilder
[ RUN ] IncrementalCompilerBuilder.SetCompilerArgs
[ OK ] IncrementalCompilerBuilder.SetCompilerArgs (32 ms)
....
....
....
[ OK ] CodeCompletionTest.NestedInvocations (44 ms)
[ RUN ] CodeCompletionTest.TemplateFunctions
[ OK ] CodeCompletionTest.TemplateFunctions (62 ms)
[----------] 12 tests from CodeCompletionTest (710 ms total)
[----------] Global test environment tear-down
[==========] 27 tests from 5 test suites ran. (2060 ms total)
[ PASSED ] 23 tests.
[ SKIPPED ] 4 tests, listed below:
[ SKIPPED ] IncrementalCompilerBuilder.SetTargetTriple
[ SKIPPED ] InterpreterTest.UndoCommand
[ SKIPPED ] InterpreterExtensionsTest.DefaultCrossJIT
[ SKIPPED ] InterpreterExtensionsTest.CustomCrossJIT
- Tried it in a demo playground
- The next step what I did is, based on my comment above #175448 (comment), I tried having a wasm build with no orc related symbols whatsoever/ not compiling any orc jit lib
A quick rough diff for that would be
diff --git a/clang/include/clang/Interpreter/IncrementalExecutor.h b/clang/include/clang/Interpreter/IncrementalExecutor.h
index 913da9230a94..be72696e4cfa 100644
--- a/clang/include/clang/Interpreter/IncrementalExecutor.h
+++ b/clang/include/clang/Interpreter/IncrementalExecutor.h
@@ -33,6 +33,7 @@ class Compilation;
class IncrementalExecutorBuilder {
public:
+#ifndef __EMSCRIPTEN__
/// Indicates whether out-of-process JIT execution is enabled.
bool IsOutOfProcess = false;
/// Path to the out-of-process JIT executor.
@@ -50,8 +51,12 @@ public:
std::function<void()> CustomizeFork = nullptr;
/// An optional code model to provide to the JITTargetMachineBuilder
std::optional<llvm::CodeModel::Model> CM = std::nullopt;
+#endif
+
/// An optional external IncrementalExecutor
std::unique_ptr<IncrementalExecutor> IE;
+
+#ifndef __EMSCRIPTEN__
/// An optional external orc jit builder
std::unique_ptr<llvm::orc::LLJITBuilder> JITBuilder;
/// A default callback that can be used in the IncrementalCompilerBuilder to
@@ -60,6 +65,7 @@ public:
UpdateOrcRuntimePathCB = [this](const driver::Compilation &C) {
return UpdateOrcRuntimePath(C);
};
+#endif
~IncrementalExecutorBuilder();
@@ -67,7 +73,9 @@ public:
create(llvm::orc::ThreadSafeContext &TSC, const clang::TargetInfo &TI);
private:
+#ifndef __EMSCRIPTEN__
llvm::Error UpdateOrcRuntimePath(const driver::Compilation &C);
+#endif
};
struct PartialTranslationUnit;
diff --git a/clang/lib/Interpreter/CMakeLists.txt b/clang/lib/Interpreter/CMakeLists.txt
index 01d3295d1ac3..50fcd99b8a08 100644
--- a/clang/lib/Interpreter/CMakeLists.txt
+++ b/clang/lib/Interpreter/CMakeLists.txt
@@ -1,17 +1,23 @@
set(LLVM_LINK_COMPONENTS
- core
- native
- MC
- Option
- OrcJit
- OrcDebugging
- OrcShared
- OrcTargetProcess
- Support
- Target
- TargetParser
- TransformUtils
- )
+ core
+ native
+ MC
+ Option
+ Support
+ Target
+ TargetParser
+ TransformUtils
+)
+
+# Only add ORC JIT-related components for non-Emscripten builds
+if(NOT EMSCRIPTEN)
+ list(APPEND LLVM_LINK_COMPONENTS
+ OrcJit
+ OrcDebugging
+ OrcShared
+ OrcTargetProcess
+ )
+endif()
if (EMSCRIPTEN AND "lld" IN_LIST LLVM_ENABLE_PROJECTS)
set(WASM_SRC Wasm.cpp)
@@ -19,12 +25,11 @@ if (EMSCRIPTEN AND "lld" IN_LIST LLVM_ENABLE_PROJECTS)
set(COMMON_LINK lldCommon)
endif()
-add_clang_library(clangInterpreter
+set(INTERPRETER_SOURCES
DeviceOffload.cpp
CodeCompletion.cpp
IncrementalAction.cpp
IncrementalExecutor.cpp
- OrcIncrementalExecutor.cpp
IncrementalParser.cpp
Interpreter.cpp
InterpreterValuePrinter.cpp
@@ -32,6 +37,15 @@ add_clang_library(clangInterpreter
Value.cpp
InterpreterValuePrinter.cpp
${WASM_SRC}
+)
+
+# Only include OrcIncrementalExecutor for non-Emscripten builds
+if(NOT EMSCRIPTEN)
+ list(APPEND INTERPRETER_SOURCES OrcIncrementalExecutor.cpp)
+endif()
+
+add_clang_library(clangInterpreter
+ ${INTERPRETER_SOURCES}
PARTIAL_SOURCES_INTENDED
DEPENDS
@@ -53,7 +67,7 @@ add_clang_library(clangInterpreter
clangSerialization
${WASM_LINK}
${COMMON_LINK}
- )
+)
if ((MINGW OR CYGWIN) AND BUILD_SHARED_LIBS)
# The DLLs are supposed to export all symbols (except for ones that are
diff --git a/clang/lib/Interpreter/IncrementalExecutor.cpp b/clang/lib/Interpreter/IncrementalExecutor.cpp
index 001651522c32..0d3c08297c1d 100644
--- a/clang/lib/Interpreter/IncrementalExecutor.cpp
+++ b/clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -11,10 +11,11 @@
//===----------------------------------------------------------------------===//
#include "clang/Interpreter/IncrementalExecutor.h"
-#include "OrcIncrementalExecutor.h"
#ifdef __EMSCRIPTEN__
-#include "Wasm.h"
-#endif // __EMSCRIPTEN__
+ #include "Wasm.h"
+#else
+ #include "OrcIncrementalExecutor.h"
+#endif
#include "clang/Basic/TargetInfo.h"
#include "clang/Driver/Compilation.h"
@@ -26,17 +27,19 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
-#include "llvm/ExecutionEngine/JITLink/JITLinkMemoryManager.h"
-#include "llvm/ExecutionEngine/Orc/Debugging/DebuggerSupport.h"
-#include "llvm/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.h"
-#include "llvm/ExecutionEngine/Orc/ExecutionUtils.h"
-#include "llvm/ExecutionEngine/Orc/ExecutorProcessControl.h"
-#include "llvm/ExecutionEngine/Orc/JITTargetMachineBuilder.h"
-#include "llvm/ExecutionEngine/Orc/LLJIT.h"
-#include "llvm/ExecutionEngine/Orc/MapperJITLinkMemoryManager.h"
-#include "llvm/ExecutionEngine/Orc/Shared/OrcRTBridge.h"
-#include "llvm/ExecutionEngine/Orc/Shared/SimpleRemoteEPCUtils.h"
-#include "llvm/ExecutionEngine/Orc/SimpleRemoteEPC.h"
+#ifndef __EMSCRIPTEN__
+ #include "llvm/ExecutionEngine/JITLink/JITLinkMemoryManager.h"
+ #include "llvm/ExecutionEngine/Orc/Debugging/DebuggerSupport.h"
+ #include "llvm/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.h"
+ #include "llvm/ExecutionEngine/Orc/ExecutionUtils.h"
+ #include "llvm/ExecutionEngine/Orc/ExecutorProcessControl.h"
+ #include "llvm/ExecutionEngine/Orc/JITTargetMachineBuilder.h"
+ #include "llvm/ExecutionEngine/Orc/LLJIT.h"
+ #include "llvm/ExecutionEngine/Orc/MapperJITLinkMemoryManager.h"
+ #include "llvm/ExecutionEngine/Orc/Shared/OrcRTBridge.h"
+ #include "llvm/ExecutionEngine/Orc/Shared/SimpleRemoteEPCUtils.h"
+ #include "llvm/ExecutionEngine/Orc/SimpleRemoteEPC.h"
+#endif
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
@@ -63,6 +66,7 @@
namespace clang {
IncrementalExecutorBuilder::~IncrementalExecutorBuilder() = default;
+#ifndef __EMSCRIPTEN__
static llvm::Expected<llvm::orc::JITTargetMachineBuilder>
createJITTargetMachineBuilder(const llvm::Triple &TT) {
if (TT.getTriple() == llvm::sys::getProcessTriple())
@@ -368,12 +372,20 @@ outOfProcessJITBuilder(const IncrementalExecutorBuilder &IncrExecutorBuilder) {
return std::make_pair(std::move(JB), childPid);
}
+#endif // !__EMSCRIPTEN__
llvm::Expected<std::unique_ptr<IncrementalExecutor>>
IncrementalExecutorBuilder::create(llvm::orc::ThreadSafeContext &TSC,
const clang::TargetInfo &TI) {
if (IE)
return std::move(IE);
+
+#ifdef __EMSCRIPTEN__
+ (void)TSC;
+ (void)TI;
+ return std::make_unique<WasmIncrementalExecutor>();
+#else
+
llvm::Triple TT = TI.getTriple();
if (!TT.isOSWindows() && IsOutOfProcess) {
if (!JITBuilder) {
@@ -403,18 +415,16 @@ IncrementalExecutorBuilder::create(llvm::orc::ThreadSafeContext &TSC,
llvm::Error Err = llvm::Error::success();
std::unique_ptr<IncrementalExecutor> Executor;
-#ifdef __EMSCRIPTEN__
- Executor = std::make_unique<WasmIncrementalExecutor>();
-#else
Executor = std::make_unique<OrcIncrementalExecutor>(TSC, *JITBuilder, Err);
-#endif
if (Err)
return std::move(Err);
return std::move(Executor);
+#endif
}
+#ifndef __EMSCRIPTEN__
llvm::Error IncrementalExecutorBuilder::UpdateOrcRuntimePath(
const clang::driver::Compilation &C) {
if (!IsOutOfProcess)
@@ -506,5 +516,6 @@ llvm::Error IncrementalExecutorBuilder::UpdateOrcRuntimePath(
llvm::formatv("OrcRuntime library not found in: {0}", Joined).str(),
llvm::inconvertibleErrorCode());
}
+#endif // !__EMSCRIPTEN__
} // end namespace clang
This wasm build on libclangInterpreter.a would be devoid of any ORC influence. And then again I ran the tests. All tests pass as expected and the binary size is much smaller as we need.
Shall raise a PR with my rough draft soon and we can review it further there.
|
Let's merge this and then have this diff (#175448 (comment)) in the commit that follows (testing it in the meantime) |
|
The original Interpreter implementation had a hard dependency on ORC and grew organically with the addition of out-of-process JIT support. This tightly coupled the Interpreter to a specific execution engine and leaked ORC-specific assumptions (runtime layout, symbol lookup, exception model) into higher layers. The WebAssembly integration demonstrated that incremental execution can be implemented without ORC, exposing the need for a cleaner abstraction boundary. This change introduces an IncrementalExecutor interface and moves ORC-based execution behind a concrete implementation. The Interpreter now depends only on the abstract executor, improving layering and encapsulation. In addition, the Interpreter can be configured with user-provided incremental executor implementations, enabling ORC-independent execution, easier testing, and future extensions without modifying the core Interpreter.
The original Interpreter implementation had a hard dependency on ORC and grew organically with the addition of out-of-process JIT support. This tightly coupled the Interpreter to a specific execution engine and leaked ORC-specific assumptions (runtime layout, symbol lookup, exception model) into higher layers.
The WebAssembly integration demonstrated that incremental execution can be implemented without ORC, exposing the need for a cleaner abstraction boundary.
This change introduces an IncrementalExecutor interface and moves ORC-based execution behind a concrete implementation. The Interpreter now depends only on the abstract executor, improving layering and encapsulation.
In addition, the Interpreter can be configured with user-provided incremental executor implementations, enabling ORC-independent execution, easier testing, and future extensions without modifying the core Interpreter.