[clang-repl] MakeJITBuilder attribute in JITConfig

[kr-2003]

Reference - #155140 (comment)

[llvmbot]

@llvm/pr-subscribers-clang

Author: Abhinav Kumar (kr-2003)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/159693.diff

2 Files Affected:

• (modified) clang/include/clang/Interpreter/Interpreter.h (+19-8)
• (modified) clang/lib/Interpreter/Interpreter.cpp (+53-53)

diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index 078d70b3b1749..ef56f319e00ef 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -1,4 +1,5 @@
 //===--- Interpreter.h - Incremental Compilation and 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.
@@ -19,12 +20,14 @@
 #include "clang/Interpreter/Value.h"
 
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ExecutionEngine/JITSymbol.h"
 #include "llvm/ExecutionEngine/Orc/ExecutorProcessControl.h"
 #include "llvm/ExecutionEngine/Orc/Shared/ExecutorAddress.h"
 #include "llvm/Support/Error.h"
 #include <cstdint>
 #include <memory>
+#include <optional>
 #include <vector>
 
 namespace llvm {
@@ -32,6 +35,7 @@ namespace orc {
 class LLJIT;
 class LLJITBuilder;
 class ThreadSafeContext;
+class JITTargetMachineBuilder;
 } // namespace orc
 } // namespace llvm
 
@@ -132,18 +136,25 @@ class Interpreter {
     /// Representing the slab allocation size for memory management in kb.
     unsigned SlabAllocateSize = 0;
     /// Path to the ORC runtime library.
-    std::string OrcRuntimePath = "";
+    std::optional<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;
+
+    /// Factory function for creating LLJITBuilder instances.
+    /// This allows clients to customize JIT builder creation while still
+    /// providing sensible defaults.
+    llvm::unique_function<llvm::Expected<
+        std::unique_ptr<llvm::orc::LLJITBuilder>>(const JITConfig &)>
+        MakeJITBuilder = makeDefaultJITBuilder;
 
     JITConfig()
         : IsOutOfProcess(false), OOPExecutor(""), OOPExecutorConnect(""),
-          UseSharedMemory(false), SlabAllocateSize(0), OrcRuntimePath(""),
-          ExecutorPID(0), CustomizeFork(nullptr), CM(std::nullopt) {}
+          UseSharedMemory(false), SlabAllocateSize(0), OrcRuntimePath(),
+          ExecutorPID(0) {}
+
+    /// Default JIT builder factory function
+    static llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>>
+    makeDefaultJITBuilder(const JITConfig &Config);
   };
 
 protected:
@@ -237,4 +248,4 @@ class Interpreter {
 };
 } // namespace clang
 
-#endif // LLVM_CLANG_INTERPRETER_INTERPRETER_H
+#endif // LLVM_CLANG_INTERPRETER_INTERPRETER_H
\ No newline at end of file
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 9cc1c450b7650..c33a037408d61 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -350,15 +350,23 @@ const char *const Runtimes = R"(
   EXTERN_C void __clang_Interpreter_SetValueNoAlloc(void *This, void *OutVal, void *OpaqueType, ...);
 )";
 
+static llvm::Expected<llvm::orc::JITTargetMachineBuilder>
+createJITTargetMachineBuilder(const std::string &TT) {
+  if (TT == llvm::sys::getProcessTriple())
+    return llvm::orc::JITTargetMachineBuilder::detectHost();
+
+  return llvm::orc::JITTargetMachineBuilder(llvm::Triple(TT));
+}
+
 llvm::Expected<std::pair<std::unique_ptr<llvm::orc::LLJITBuilder>, uint32_t>>
 Interpreter::outOfProcessJITBuilder(JITConfig Config) {
   std::unique_ptr<llvm::orc::ExecutorProcessControl> EPC;
   uint32_t childPid = -1;
+
   if (!Config.OOPExecutor.empty()) {
     // Launch an out-of-process executor locally in a child process.
     auto ResultOrErr = IncrementalExecutor::launchExecutor(
-        Config.OOPExecutor, Config.UseSharedMemory, Config.SlabAllocateSize,
-        Config.CustomizeFork);
+        Config.OOPExecutor, Config.UseSharedMemory, Config.SlabAllocateSize);
     if (!ResultOrErr)
       return ResultOrErr.takeError();
     childPid = ResultOrErr->second;
@@ -381,8 +389,10 @@ Interpreter::outOfProcessJITBuilder(JITConfig Config) {
 
   std::unique_ptr<llvm::orc::LLJITBuilder> JB;
   if (EPC) {
-    auto JBOrErr = clang::Interpreter::createLLJITBuilder(
-        std::move(EPC), Config.OrcRuntimePath);
+    std::string RuntimePath =
+        Config.OrcRuntimePath ? *Config.OrcRuntimePath : "";
+    auto JBOrErr =
+        clang::Interpreter::createLLJITBuilder(std::move(EPC), RuntimePath);
     if (!JBOrErr)
       return JBOrErr.takeError();
     JB = std::move(*JBOrErr);
@@ -422,9 +432,7 @@ llvm::Expected<std::unique_ptr<Interpreter>>
 Interpreter::create(std::unique_ptr<CompilerInstance> CI, JITConfig Config) {
   llvm::Error Err = llvm::Error::success();
 
-  std::unique_ptr<llvm::orc::LLJITBuilder> JB;
-
-  if (Config.IsOutOfProcess) {
+  if (Config.IsOutOfProcess && !Config.OrcRuntimePath) {
     const TargetInfo &TI = CI->getTarget();
     const llvm::Triple &Triple = TI.getTriple();
 
@@ -440,16 +448,21 @@ Interpreter::create(std::unique_ptr<CompilerInstance> CI, JITConfig Config) {
           "Failed to create driver compilation for out-of-process JIT",
           std::error_code());
     }
-    if (Config.OrcRuntimePath == "") {
-      const clang::driver::ToolChain &TC = C->getDefaultToolChain();
+    const clang::driver::ToolChain &TC = C->getDefaultToolChain();
+    auto OrcRuntimePathOrErr = getOrcRuntimePath(TC);
+    if (!OrcRuntimePathOrErr) {
+      return OrcRuntimePathOrErr.takeError();
+    }
 
-      auto OrcRuntimePathOrErr = getOrcRuntimePath(TC);
-      if (!OrcRuntimePathOrErr) {
-        return OrcRuntimePathOrErr.takeError();
-      }
+    Config.OrcRuntimePath = *OrcRuntimePathOrErr;
+  }
 
-      Config.OrcRuntimePath = *OrcRuntimePathOrErr;
-    }
+  std::unique_ptr<llvm::orc::LLJITBuilder> JB;
+  if (Config.IsOutOfProcess) {
+    auto JBOrErr = Config.MakeJITBuilder(Config);
+    if (!JBOrErr)
+      return JBOrErr.takeError();
+    JB = std::move(*JBOrErr);
   }
 
   auto Interp = std::unique_ptr<Interpreter>(new Interpreter(
@@ -590,16 +603,6 @@ Interpreter::Parse(llvm::StringRef Code) {
   return LastPTU;
 }
 
-static llvm::Expected<llvm::orc::JITTargetMachineBuilder>
-createJITTargetMachineBuilder(const std::string &TT) {
-  if (TT == 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(llvm::Triple(TT));
-}
-
 llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>>
 Interpreter::createLLJITBuilder(
     std::unique_ptr<llvm::orc::ExecutorProcessControl> EPC,
@@ -629,39 +632,16 @@ llvm::Error Interpreter::CreateExecutor(JITConfig Config) {
                                                "No code generator available",
                                                std::error_code());
 
-  const std::string &TT = getCompilerInstance()->getTargetOpts().Triple;
-  llvm::Triple TargetTriple(TT);
-  bool IsWindowsTarget = TargetTriple.isOSWindows();
-
-  if (!IsWindowsTarget && Config.IsOutOfProcess) {
-    if (!JITBuilder) {
-      auto ResOrErr = outOfProcessJITBuilder(Config);
-      if (!ResOrErr)
-        return ResOrErr.takeError();
-      JITBuilder = std::move(ResOrErr->first);
-      Config.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 (Config.CM)
-      JTMB->setCodeModel(Config.CM);
-    auto JB = IncrementalExecutor::createDefaultJITBuilder(std::move(*JTMB));
-    if (!JB)
-      return JB.takeError();
-    JITBuilder = std::move(*JB);
+    // Use the factory function to create the builder
+    auto JBOrErr = Config.MakeJITBuilder(Config);
+    if (!JBOrErr)
+      return JBOrErr.takeError();
+    JITBuilder = std::move(*JBOrErr);
   }
 
   llvm::Error Err = llvm::Error::success();
 
-  // Fix: Declare Executor as the appropriate unique_ptr type
   std::unique_ptr<IncrementalExecutor> Executor;
 
 #ifdef __EMSCRIPTEN__
@@ -803,4 +783,24 @@ llvm::Error Interpreter::LoadDynamicLibrary(const char *name) {
 
   return llvm::Error::success();
 }
+
+llvm::Expected<std::unique_ptr<llvm::orc::LLJITBuilder>>
+Interpreter::JITConfig::makeDefaultJITBuilder(const JITConfig &Config) {
+  // Handle out-of-process JIT case
+  if (Config.IsOutOfProcess) {
+    if (!Config.OOPExecutor.empty() || !Config.OOPExecutorConnect.empty()) {
+      auto ResOrErr = Interpreter::outOfProcessJITBuilder(Config);
+      if (!ResOrErr)
+        return ResOrErr.takeError();
+      return std::move(ResOrErr->first);
+    }
+  }
+
+  std::string TT = llvm::sys::getProcessTriple();
+  auto JTMB = createJITTargetMachineBuilder(TT);
+  if (!JTMB)
+    return JTMB.takeError();
+
+  return IncrementalExecutor::createDefaultJITBuilder(std::move(*JTMB));
+}
 } // end namespace clang

[kr-2003]

@vgvassilev @lhames Can you review this?