[Clang-Repl] Sinking RemoteJITUtils into Interpreter class(Refactoring) #155140

kr-2003 · 2025-08-24T05:03:06Z

This is a refactoring PR. It sinks RemoteJITUtils into Interpreter and IncrementalExecutor classes.

…into redirection

…rection

vgvassilev · 2025-08-25T09:35:35Z

clang/tools/clang-repl/ClangRepl.cpp

+    if (::OffloadArch.empty()) {
+      ::OffloadArch = "sm_35";
    }
-    CB.SetOffloadArch(OffloadArch);
+    CB.SetOffloadArch(::OffloadArch);


Are these changes relevant to the current PR?

yes. if not done, it gives following error:

error: reference to 'OffloadArch' is ambiguous

Which compiler is this? Why it works upstream?

(base) llvm-project-test % clang --version Homebrew clang version 20.1.6 Target: arm64-apple-darwin24.5.0 Thread model: posix InstalledDir: /opt/homebrew/Cellar/llvm/20.1.6/bin Configuration file: /opt/homebrew/etc/clang/arm64-apple-darwin24.cfg'

clang/tools/clang-repl/ClangRepl.cpp

github-actions · 2025-08-25T14:47:58Z

✅ With the latest revision this PR passed the C/C++ code formatter.

clang/tools/clang-repl/ClangRepl.cpp

clang/lib/Interpreter/RemoteJITUtils.cpp

clang/unittests/Interpreter/OutOfProcessInterpreterTests.cpp

clang/include/clang/Interpreter/OutOfProcessJITConfig.h

vgvassilev · 2025-08-28T07:50:36Z

clang/include/clang/Interpreter/OutOfProcessJITConfig.h

+  bool UseSharedMemory;
+
+  /// String representing the slab allocation size for memory management.
+  std::string SlabAllocateSizeString;


This should be an unsigned int I think. All of these should have good default values.

Actually this has to be string. this is how it is handled:

static llvm::cl::opt<std::string> SlabAllocateSizeString( "slab-allocate", llvm::cl::desc("Allocate from a slab of the given size " "(allowable suffixes: Kb, Mb, Gb. default = " "Kb)"), llvm::cl::init(""), llvm::cl::cat(OOPCategory));

it has suffix

We can transform it before putting it in the config.

clang/lib/Interpreter/Interpreter.cpp

clang/lib/Interpreter/RemoteJITUtils.cpp

clang/tools/clang-repl/ClangRepl.cpp

vgvassilev · 2025-08-28T07:56:18Z

clang/tools/clang-repl/ClangRepl.cpp

+    if (::OffloadArch.empty()) {
+      ::OffloadArch = "sm_35";
    }
-    CB.SetOffloadArch(OffloadArch);
+    CB.SetOffloadArch(::OffloadArch);


Which compiler is this? Why it works upstream?

clang/tools/clang-repl/ClangRepl.cpp

…into redirection

clang/include/clang/Interpreter/Interpreter.h

vgvassilev · 2025-09-02T06:49:13Z

clang/include/clang/Interpreter/Interpreter.h

+    /// Indicates whether to use shared memory for communication.
+    bool UseSharedMemory;
+    /// String representing the slab allocation size for memory management.
+    std::string SlabAllocateSizeString;


Suggested change

std::string SlabAllocateSizeString;

unsigned SlabAllocateSize = 1024;

I still believe that having this as string should not be changed. otherwise, we will have more code for conversion

The code for conversion will be exactly the same. It's location will be different. I do not see a reason for the user API to take strings when it can take the right types constructed programmatically.

vgvassilev · 2025-09-02T06:49:23Z

clang/include/clang/Interpreter/Interpreter.h

+    std::string OOPExecutor;
+    std::string OOPExecutorConnect;
+    /// Indicates whether to use shared memory for communication.
+    bool UseSharedMemory;


Suggested change

bool UseSharedMemory;

bool UseSharedMemory = true;

already initialised in the constructor

It's probably clearer to initialize it here too.

vgvassilev · 2025-09-02T06:49:34Z

clang/include/clang/Interpreter/Interpreter.h

+public:
+  struct OutOfProcessJITConfig {
+    /// Indicates whether out-of-process JIT execution is enabled.
+    bool IsOutOfProcess;


Suggested change

bool IsOutOfProcess;

bool IsOutOfProcess = false;

clang/lib/Interpreter/Interpreter.cpp

vgvassilev · 2025-09-02T07:01:48Z

clang/lib/Interpreter/Interpreter.cpp

@@ -535,9 +659,14 @@ llvm::Error Interpreter::CreateExecutor() {
  llvm::Error Err = llvm::Error::success();
 #ifdef __EMSCRIPTEN__
  auto Executor = std::make_unique<WasmIncrementalExecutor>(*TSCtx);
+#else
+#ifndef _WIN32


We don't need a special branch -- we just need to pass in empty JitConfig.

vgvassilev · 2025-09-05T06:09:01Z

clang/lib/Interpreter/Interpreter.cpp

+    DiagnosticsEngine &Diags = CI->getDiagnostics();
+    std::string BinaryName = llvm::sys::fs::getMainExecutable(nullptr, nullptr);
+    driver::Driver Driver(BinaryName, Triple.str(), Diags);
+    std::vector<const char *> Args = {"clang", "--version"};
+    std::unique_ptr<clang::driver::Compilation> C(
+        Driver.BuildCompilation(Args));
+    if (!C) {
+      return llvm::make_error<llvm::StringError>(
+          "Failed to create driver compilation for out-of-process JIT",
+          std::error_code());
+    }
+    if (Config.OrcRuntimePath == "") {
+      const clang::driver::ToolChain &TC = C->getDefaultToolChain();
+
+      auto OrcRuntimePathOrErr = getOrcRuntimePath(TC);
+      if (!OrcRuntimePathOrErr) {
+        return OrcRuntimePathOrErr.takeError();
+      }
+
+      Config.OrcRuntimePath = *OrcRuntimePathOrErr;
+    }


This needs to happen on the existing compiler instance that we create and not on a fake one clang --version.

vgvassilev · 2025-09-05T06:09:36Z

clang/lib/Interpreter/Interpreter.cpp

@@ -521,6 +618,23 @@ llvm::Error Interpreter::CreateExecutor() {
    return llvm::make_error<llvm::StringError>("Operation failed. "
                                               "No code generator available",
                                               std::error_code());
+#ifndef _WIN32


We must check the target triple, not an ifdef because we can be cross compiling.

kr-2003 and others added 21 commits July 29, 2025 13:34

pipes for redirection in oop jit

79849c3

Merge branch 'main' into redirection

825f005

refactoring

52b0902

refactoring

3b4fcad

commenting & refactoring

e174e98

compiler-rt conditional addition

b744a9f

Merge branch 'main' into redirection

0c6c995

removed compiler-rt dep

2cf71ed

Merge branch 'redirection' of https://github.com/kr-2003/llvm-project …

7c29008

…into redirection

separate file for oop tests

41f8e54

separate file for oop tests

4f1d203

separate file for oop tests

a6a4369

test file rename

63de886

test file rename

c24e84e

test file rename

b114bb8

resolving comments & InterpreterRemoteTest

850b953

resolving comments

956f393

resolving comments

14e5afd

Custom lambda in launchExecutor and pid retrieval

65afbff

Merge branch 'main' of https://github.com/llvm/llvm-project into redi…

7dc6590

…rection

Dynamic path resolution

095a63b

vgvassilev reviewed Aug 25, 2025

View reviewed changes

vgvassilev requested a review from lhames August 25, 2025 12:07

kr-2003 added 2 commits August 25, 2025 18:52

Dynamic path resolution using IncrementalCB

2b6dc6c

Dynamic path resolution using IncrementalCB

7ad10a6

vgvassilev reviewed Aug 25, 2025

View reviewed changes

clang/tools/clang-repl/ClangRepl.cpp Outdated Show resolved Hide resolved

vgvassilev reviewed Aug 25, 2025

View reviewed changes

clang/lib/Interpreter/RemoteJITUtils.cpp Outdated Show resolved Hide resolved

Addressing reviews

6a58d5f

kr-2003 requested a review from vgvassilev August 26, 2025 15:12

vgvassilev reviewed Aug 26, 2025

View reviewed changes

clang/unittests/Interpreter/OutOfProcessInterpreterTests.cpp Outdated Show resolved Hide resolved

Sinking JitBuilder into Interpreter

f1b9135

kr-2003 requested a review from vgvassilev August 27, 2025 16:12

vgvassilev reviewed Aug 28, 2025

View reviewed changes

kr-2003 added 3 commits August 31, 2025 19:01

RemoteJITUtils shifted to IncrementalExecutor

dd73052

RemoteJITUtils shifted to IncrementalExecutor

e90df55

Removed custom fork in launchExecutor lambda

521be31

kr-2003 force-pushed the redirection branch from fda875f to 521be31 Compare August 31, 2025 18:08

Merge branch 'main' into redirection

baaff1e

kr-2003 changed the title ~~[Clang-Repl] Add Lambda Support, PID Retrieval, and Dynamic liborc_rt Path in Clang-Repl~~ [Clang-Repl] Sinking RemoteJITUtils into Interpreter class(Refactoring) Aug 31, 2025

kr-2003 added 2 commits August 31, 2025 23:43

refactoring changes

44cde65

Merge branch 'redirection' of https://github.com/kr-2003/llvm-project …

fc1f0da

…into redirection

kr-2003 requested a review from vgvassilev August 31, 2025 18:18

kr-2003 added 7 commits September 1, 2025 00:21

Refactoring

c49ec55

Deleted RemoteJITUtils.h & .cpp

a6a05ea

Refactoring

f2ca64e

Refactoring

0c99b67

Refactoring

51d6657

formatting

a71c546

formatting

37a9f3d

vgvassilev reviewed Sep 2, 2025

View reviewed changes

kr-2003 added 2 commits September 5, 2025 09:57

Addressing reviews

2cdb210

Addressing reviews

129a62b

vgvassilev reviewed Sep 5, 2025

View reviewed changes

	std::string SlabAllocateSizeString;
	unsigned SlabAllocateSize = 1024;

[Clang-Repl] Sinking RemoteJITUtils into Interpreter class(Refactoring) #155140

Are you sure you want to change the base?

[Clang-Repl] Sinking RemoteJITUtils into Interpreter class(Refactoring) #155140

Conversation

kr-2003 commented Aug 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kr-2003 commented Aug 24, 2025 •

edited

Loading

github-actions bot commented Aug 25, 2025 •

edited

Loading