Skip to content

Commit f64ddbe

Browse files
authored
[ORCJIT] Reinject contexts on first function lookup (#655)
## Summary - defer context injection from empty-library construction to the first ordinary function lookup - cache each successful refresh; after LLVM accepts an object, AddObjectFile publishes the refresh flag as false - keep AddObjectFile outside the thread-safety contract: it must not race GetFunction, and a failed add naturally leaves the prior flag unchanged - keep concurrent GetFunction calls thread-safe with an acquire fast path and a mutex-protected, double-checked refresh on the false path, followed by one ordinary symbol lookup - preserve ordinary GetSymbol initializer execution and LLVM error propagation - keep the ORCJIT wheel test environment aligned with the repository-wide parallel pytest configuration by installing pytest-xdist - bump the addon package to 0.1.1 If callers violate the AddObjectFile/GetFunction contract, LLVM may expose a newly added symbol and GetFunction may return its stable allocated address before that object's context slots are refreshed. The symbol remains visible, but context initialization is not guaranteed for that unsupported overlap. ## Regression One C object defines only __tvm_ffi__library_ctx and a probe. Barrier-synchronized first lookups verify that every returned function observes the injected pointer without adding another fixture. ## Validation - GCC- and Clang-built addon suites: 139 passed, 3 platform skips each - fresh CI-equivalent wheel environment with local root package, default parallel pytest, and C/C++ quick starts - 200 rounds with eight concurrent first GetFunction callers after object addition - focused context, invalid-object, constructor, and destructor tests - fresh 0.1.1 wheel build, isolated install, focused regression, and quick starts - applicable formatting and lint hooks
1 parent 5411a64 commit f64ddbe

9 files changed

Lines changed: 103 additions & 14 deletions

File tree

.github/actions/build-orcjit-wheel/action.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ runs:
102102
CIBW_ENVIRONMENT: LLVM_PREFIX=/opt/llvm
103103
CIBW_ENVIRONMENT_WINDOWS: LLVM_PREFIX="C:/opt/llvm"
104104
CIBW_CONTAINER_ENGINE: "docker; create_args: --volume /opt/llvm:/opt/llvm"
105-
CIBW_TEST_REQUIRES: pytest ninja
105+
CIBW_TEST_REQUIRES: pytest pytest-xdist ninja
106106
CIBW_TEST_COMMAND: >-
107107
pip install --force-reinstall {project} &&
108108
pytest {project}/addons/tvm_ffi_orcjit/tests -v &&

addons/tvm_ffi_orcjit/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
cmake_minimum_required(VERSION 3.20)
22
project(
33
tvm_ffi_orcjit
4-
VERSION 0.1.0
4+
VERSION 0.1.1
55
LANGUAGES C CXX
66
)
77

addons/tvm_ffi_orcjit/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ build-backend = "scikit_build_core.build"
2121

2222
[project]
2323
name = "apache-tvm_ffi_orcjit"
24-
version = "0.1.0"
24+
version = "0.1.1"
2525
description = "Load TVM-FFI exported object files using LLVM ORC JIT v2"
2626
readme = "README.md"
2727
requires-python = ">=3.10"

addons/tvm_ffi_orcjit/src/ffi/orcjit_dylib.cc

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,6 @@ ORCJITDynamicLibraryObj::ORCJITDynamicLibraryObj(ORCJITExecutionSession session,
6868
: session_(std::move(session)), dylib_(dylib), jit_(jit), name_(std::move(name)) {
6969
TVM_FFI_CHECK(dylib_ != nullptr, ValueError) << "JITDylib cannot be null";
7070
TVM_FFI_CHECK(jit_ != nullptr, ValueError) << "LLJIT cannot be null";
71-
if (void** ctx_addr = reinterpret_cast<void**>(GetSymbol(ffi::symbol::tvm_ffi_library_ctx))) {
72-
*ctx_addr = this;
73-
}
74-
Module::VisitContextSymbols([this](const ffi::String& name, void* symbol) {
75-
if (void** ctx_addr = reinterpret_cast<void**>(GetSymbol(name))) {
76-
*ctx_addr = symbol;
77-
}
78-
});
7971
}
8072

8173
ORCJITDynamicLibraryObj::~ORCJITDynamicLibraryObj() {
@@ -111,8 +103,9 @@ void ORCJITDynamicLibraryObj::AddObjectFile(const String& path) {
111103
TVM_FFI_THROW(IOError) << "Failed to read object file: " << path;
112104
}
113105

114-
// Add object file to this JITDylib
106+
// AddObjectFile is not thread-safe and must not race GetFunction.
115107
TVM_FFI_ORCJIT_LLVM_CALL(jit_->addObjectFile(*dylib_, std::move(*buffer_or_err)));
108+
context_symbol_refreshed_.store(false, std::memory_order_release);
116109
}
117110

118111
void ORCJITDynamicLibraryObj::SetLinkOrder(const std::vector<llvm::orc::JITDylib*>& dylibs) {
@@ -157,8 +150,29 @@ void* ORCJITDynamicLibraryObj::GetSymbol(const String& name) {
157150
CxaAtexitRecordsScope scope(&cxa_atexit_records_);
158151
#endif
159152
session_->RunPendingInitializers(GetJITDylib());
160-
// Convert ExecutorAddr to pointer
161-
return symbol_or_err ? symbol_or_err->getAddress().toPtr<void*>() : nullptr;
153+
154+
if (!symbol_or_err) {
155+
llvm::Error remaining =
156+
llvm::handleErrors(symbol_or_err.takeError(), [](const llvm::orc::SymbolsNotFound&) {});
157+
if (remaining) TVM_FFI_ORCJIT_LLVM_CALL(std::move(remaining));
158+
return nullptr;
159+
}
160+
return symbol_or_err->getAddress().toPtr<void*>();
161+
}
162+
163+
void ORCJITDynamicLibraryObj::InitContextSymbols() {
164+
std::lock_guard<std::mutex> lock(context_symbol_refresh_mutex_);
165+
if (context_symbol_refreshed_.load(std::memory_order_acquire)) return;
166+
167+
if (void** ctx_addr = reinterpret_cast<void**>(GetSymbol(symbol::tvm_ffi_library_ctx))) {
168+
*ctx_addr = this;
169+
}
170+
Module::VisitContextSymbols([this](const String& name, void* symbol) {
171+
if (void** ctx_addr = reinterpret_cast<void**>(GetSymbol(name))) {
172+
*ctx_addr = symbol;
173+
}
174+
});
175+
context_symbol_refreshed_.store(true, std::memory_order_release);
162176
}
163177

164178
llvm::orc::JITDylib& ORCJITDynamicLibraryObj::GetJITDylib() {
@@ -184,6 +198,10 @@ Optional<Function> ORCJITDynamicLibraryObj::GetFunction(const String& name) {
184198
// TVM-FFI exports have __tvm_ffi_ prefix
185199
std::string symbol_name = symbol::tvm_ffi_symbol_prefix + std::string(name);
186200

201+
if (!context_symbol_refreshed_.load(std::memory_order_acquire)) {
202+
InitContextSymbols();
203+
}
204+
187205
// Try to get the symbol - return NullOpt if not found
188206
if (void* symbol = GetSymbol(symbol_name)) {
189207
TVMFFISafeCallType c_func = reinterpret_cast<TVMFFISafeCallType>(symbol);

addons/tvm_ffi_orcjit/src/ffi/orcjit_dylib.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@
3030
#include <tvm/ffi/object.h>
3131
#include <tvm/ffi/string.h>
3232

33+
#include <atomic>
34+
#include <mutex>
35+
3336
#include "llvm_patches/macho_cxa_atexit_shim.h"
3437
#include "orcjit_session.h"
3538

@@ -73,6 +76,9 @@ class ORCJITDynamicLibraryObj : public ModuleObj {
7376
*/
7477
void SetLinkOrder(const std::vector<llvm::orc::JITDylib*>& dylibs);
7578

79+
/*! \brief Refresh context slots after an object add when needed. */
80+
void InitContextSymbols();
81+
7682
/*!
7783
* \brief Look up a symbol in this library
7884
* \param name The symbol name to look up
@@ -107,6 +113,12 @@ class ORCJITDynamicLibraryObj : public ModuleObj {
107113
/*! \brief Link order tracking (to support incremental linking) */
108114
llvm::orc::JITDylibSearchOrder link_order_;
109115

116+
/*! \brief Whether context slots have been refreshed since the last object add. */
117+
std::atomic<bool> context_symbol_refreshed_{false};
118+
119+
/*! \brief Serializes the false-path context refresh between function lookups. */
120+
std::mutex context_symbol_refresh_mutex_;
121+
110122
#ifdef __APPLE__
111123
/*! \brief Per-dylib __cxa_atexit registry.
112124
*

addons/tvm_ffi_orcjit/tests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ add_test_object(sources/c/test_funcs.c)
9292
add_test_object(sources/c/test_funcs2.c)
9393
add_test_object(sources/c/test_funcs_conflict.c)
9494
add_test_object(sources/c/test_call_global.c)
95+
add_test_object(sources/c/test_context.c)
9596
add_test_object(sources/c/test_types.c)
9697
add_test_object(sources/c/test_link_order_base.c)
9798
add_test_object(sources/c/test_link_order_caller.c)

addons/tvm_ffi_orcjit/tests/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ variant subdirectories (c/, cc/, c-gcc/, etc.).
8787
| `test_funcs2` | More arithmetic (subtract, divide) |
8888
| `test_funcs_conflict` | Symbol conflict testing (duplicate `add`) |
8989
| `test_call_global` | Callbacks into Python-registered global functions |
90+
| `test_context` | First-lookup library-context injection |
9091
| `test_types` | Zero-arg, multi-arg, float, void return types |
9192
| `test_link_order_base` / `test_link_order_caller` | Cross-library symbol resolution |
9293
| `test_error` | Error propagation from JIT'd code |
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#include <tvm/ffi/c_api.h>
21+
22+
TVM_FFI_DLL_EXPORT void* __tvm_ffi__library_ctx = NULL;
23+
24+
TVM_FFI_DLL_EXPORT int __tvm_ffi_context_is_set(void* self, const TVMFFIAny* args, int32_t num_args,
25+
TVMFFIAny* result) {
26+
(void)self;
27+
(void)args;
28+
(void)num_args;
29+
result->type_index = kTVMFFIInt;
30+
result->zero_padding = 0;
31+
result->v_int64 = __tvm_ffi__library_ctx != NULL;
32+
return 0;
33+
}

addons/tvm_ffi_orcjit/tests/test_basic.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import gc
2222
import sys
2323
import tempfile
24+
import threading
25+
from concurrent.futures import ThreadPoolExecutor
2426
from pathlib import Path
2527

2628
import pytest
@@ -295,6 +297,28 @@ def test_call_global(v: Variant) -> None:
295297
assert mul_func(11, 11) == 121
296298

297299

300+
# ---------------------------------------------------------------------------
301+
# Library-context injection
302+
# ---------------------------------------------------------------------------
303+
304+
305+
def test_context_injected_after_object_add() -> None:
306+
"""Concurrent first lookups after an object add observe the injected context pointer."""
307+
session = ExecutionSession()
308+
lib = session.create_library("context_after_add")
309+
lib.add(obj("c/test_context"))
310+
311+
num_workers = 8
312+
barrier = threading.Barrier(num_workers)
313+
314+
def lookup(_worker: int) -> int:
315+
barrier.wait()
316+
return lib.get_function("context_is_set")()
317+
318+
with ThreadPoolExecutor(max_workers=num_workers) as pool:
319+
assert list(pool.map(lookup, range(num_workers))) == [1] * num_workers
320+
321+
298322
# ---------------------------------------------------------------------------
299323
# Error handling — pure Python (Group 1)
300324
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)