Skip to content

[ORCJIT] Reinject contexts on first function lookup#655

Merged
cyx-6 merged 6 commits into
apache:mainfrom
tqchen:fix-orcjit-context-materialization
Jul 6, 2026
Merged

[ORCJIT] Reinject contexts on first function lookup#655
cyx-6 merged 6 commits into
apache:mainfrom
tqchen:fix-orcjit-context-materialization

Conversation

@tqchen

@tqchen tqchen commented Jul 5, 2026

Copy link
Copy Markdown
Member

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

@tqchen tqchen requested a review from cyx-6 July 5, 2026 02:07

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request defers the initialization of library context symbols in the ORC JIT compiler to the first function lookup after an object file is added, rather than performing it immediately in the constructor. It also updates error handling in symbol resolution and adds corresponding tests. The reviewer feedback correctly identifies that the newly introduced state variable context_symbol_refreshed_ is accessed and modified concurrently without synchronization, which can lead to data races. Introducing a mutex to protect this state is highly recommended to ensure thread safety.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread addons/tvm_ffi_orcjit/src/ffi/orcjit_dylib.h
Comment thread addons/tvm_ffi_orcjit/src/ffi/orcjit_dylib.cc Outdated
Comment thread addons/tvm_ffi_orcjit/src/ffi/orcjit_dylib.cc Outdated
@tqchen tqchen force-pushed the fix-orcjit-context-materialization branch from f1c66ba to c1c1484 Compare July 5, 2026 17:08
@cyx-6 cyx-6 merged commit f64ddbe into apache:main Jul 6, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants