-
Notifications
You must be signed in to change notification settings - Fork 35
[OrcJIT] Add LLVM ORC JIT v2 dynamic object loader addon #254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @yaoyaoding, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant new capability to TVM-FFI by integrating LLVM's ORC JIT v2. This integration enables the dynamic loading and execution of C++ object files directly from Python, offering a flexible and efficient way to extend TVM-FFI's functionality at runtime. The new addon provides a clear Python API for managing JIT compilation contexts and dynamic libraries, making it easier for developers to incorporate custom C++ logic without requiring a full recompile of their applications. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
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 introduces a new addon package, tvm-ffi-orcjit, to enable dynamic loading of object files using LLVM's ORC JIT v2. The overall design is robust, with a clear separation between the C++ backend and the Python frontend. The implementation demonstrates a deep understanding of LLVM's JIT capabilities, including a clever workaround for the __dso_handle issue. The tests are comprehensive and cover important scenarios like symbol conflicts.
I have identified a few areas for improvement, primarily concerning inconsistencies in documentation, build scripts, and a potential bug in the CMake configuration. There is also a mismatch between a Python API and its C++ backend. Addressing these points will enhance the quality and usability of this new addon.
addons/tvm-ffi-orcjit/CMakeLists.txt
Outdated
| target_link_libraries( | ||
| tvm_ffi_orcjit | ||
| PUBLIC tvm_ffi | ||
| PRIVATE LLVM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def link_against(self, *libraries: DynamicLibrary) -> None: | ||
| """Link this library against other dynamic libraries. | ||
|
|
||
| Sets the search order for symbol resolution. Symbols not found in this library | ||
| will be searched in the linked libraries in the order specified. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| *libraries : DynamicLibrary | ||
| One or more dynamic libraries to link against. | ||
|
|
||
| Examples | ||
| -------- | ||
| >>> session = create_session() | ||
| >>> lib_utils = session.create_library() | ||
| >>> lib_utils.add("utils.o") | ||
| >>> lib_main = session.create_library() | ||
| >>> lib_main.add("main.o") | ||
| >>> lib_main.link_against(lib_utils) # main can call utils symbols | ||
|
|
||
| """ | ||
| handles = [lib._handle for lib in libraries] | ||
| self._link_func(self._handle, *handles) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link_against method accepts *libraries, which implies it can link against multiple libraries. However, the C++ backend function orcjit.DynamicLibraryLinkAgainst only supports linking against a single library at a time. This will cause a TypeError if more than one library is passed.
To fix this, you should either update the Python method to accept only one library or modify the C++ backend to handle multiple libraries. Given the current C++ implementation, changing the Python API is the most direct solution.
def link_against(self, library: DynamicLibrary) -> None:
"""Link this library against another dynamic library.
Sets the search order for symbol resolution. Symbols not found in this library
will be searched in the linked library.
Parameters
----------
library : DynamicLibrary
The dynamic library to link against.
Examples
--------
>>> session = create_session()
>>> lib_utils = session.create_library()
>>> lib_utils.add("utils.o")
>>> lib_main = session.create_library()
>>> lib_main.add("main.o")
>>> lib_main.link_against(lib_utils) # main can call utils symbols
"""
self._link_func(self._handle, library._handle)
addons/tvm-ffi-orcjit/CMakeLists.txt
Outdated
| separate_arguments(LLVM_DEFINITIONS_LIST NATIVE_COMMAND ${LLVM_DEFINITIONS}) | ||
| add_definitions(${LLVM_DEFINITIONS_LIST}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addons/tvm-ffi-orcjit/README.md
Outdated
| from tvm_ffi_orcjit import ObjectLoader | ||
|
|
||
| # Create a loader instance | ||
| loader = ObjectLoader() | ||
|
|
||
| # Load an object file | ||
| loader.load("example.o") | ||
|
|
||
| # Get and call a function | ||
| add_func = loader.get_function("simple_add") | ||
| result = add_func(1, 2) | ||
| print(f"Result: {result}") # Output: Result: 3 | ||
| ``` | ||
|
|
||
| ### Incremental Loading | ||
|
|
||
| Load multiple object files and access functions from all of them: | ||
|
|
||
| ```python | ||
| from tvm_ffi_orcjit import ObjectLoader | ||
|
|
||
| loader = ObjectLoader() | ||
|
|
||
| # Load first object file | ||
| loader.load("math_ops.o") | ||
| add = loader.get_function("simple_add") | ||
|
|
||
| # Load second object file - functions from first remain accessible | ||
| loader.load("string_ops.o") | ||
| concat = loader.get_function("string_concat") | ||
|
|
||
| # Both functions work | ||
| print(add(10, 20)) # From math_ops.o | ||
| print(concat("Hello", "World")) # From string_ops.o | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The usage examples in this README appear to be outdated. They refer to an ObjectLoader class which is not present in the current implementation. The correct API, as demonstrated in examples/quick-start/run.py, uses create_session, session.create_library, and lib.add(). Please update the examples to reflect the current API to avoid confusion for new users.
addons/tvm-ffi-orcjit/README.md
Outdated
| ### Direct Module Access | ||
|
|
||
| You can also use TVM-FFI's `load_module` directly (`.o` files are automatically handled): | ||
|
|
||
| ```python | ||
| import tvm_ffi | ||
|
|
||
| # Load object file as a module | ||
| module = tvm_ffi.load_module("example.o") | ||
|
|
||
| # Get function | ||
| func = module.get_function("my_function") | ||
| result = func(arg1, arg2) | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section on "Direct Module Access" suggests that tvm_ffi.load_module("example.o") is supported. However, the implementation does not seem to register a module loader for .o files. If this feature is not yet implemented, it would be best to remove this section or clearly mark it as a future capability to prevent user confusion.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| int (*)(void* handle, const TVMFFIAny* args, int32_t num_args, TVMFFIAny* rv); | ||
| auto c_func = reinterpret_cast<TVMFFISafeCallType>(symbol); | ||
|
|
||
| return Function::FromPacked([c_func, name](PackedArgs args, Any* rv) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refer to existing LibraryModule impl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // Try to get the symbol - return NullOpt if not found | ||
| void* symbol = nullptr; | ||
| try { | ||
| symbol = dylib_->GetSymbol(symbol_name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return nullptr instead when symbol is not found
| from .session import ExecutionSession | ||
|
|
||
|
|
||
| class DynamicLibrary: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make it wrap ffi.Module, so we can directly do lib.foo
| self._link_func = get_global_func("orcjit.DynamicLibraryLinkAgainst") | ||
| self._to_module_func = get_global_func("orcjit.DynamicLibraryToModule") | ||
|
|
||
| def add(self, object_file: str | Path) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lib.add_object_file(path) => lib.mod["__add_object_file__"](path)
lib.foo
lib.function
| object_file = str(object_file) | ||
| self._add_func(self._handle, object_file) | ||
|
|
||
| def link_against(self, *libraries: DynamicLibrary) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lib.set_link_order(list[libraries])
| include LICENSE | ||
| include pyproject.toml | ||
| include CMakeLists.txt | ||
| recursive-include include *.h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manifest is not needed and only need in pyproject
|
One general question, how do we plan to manage the versions of these add ons? |
|
I think most likely they can evolve independently for now and optional |
This PR introduces a new addon package
tvm-ffi-orcjitthat enables dynamic loading of TVM-FFI exported object files (.o) at runtime using LLVM's ORC JIT v2 engine.The addon provides a Python API for loading compiled object files, load the tvm-ffi functions defined in the object files.
The API is organized around three main concepts:
.ofiles containing TVM-FFI exported functions.Usage Example
For incremental loading, you can add multiple object files to the same session:
See the test for more example.
TODO