-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[Runtime] Allow inspection of function names from a compiled .so #16836
base: main
Are you sure you want to change the base?
[Runtime] Allow inspection of function names from a compiled .so #16836
Conversation
seems this behavior was platform dependent, and ideally we should not introduce behavior that only works on one platform but not others. It is also hard for other implementations of the same Module interface to support this behavior. |
src/runtime/dso_library.cc
Outdated
struct link_map* map = nullptr; | ||
dlinfo(lib_handle_, RTLD_DI_LINKMAP, &map); | ||
|
||
Elf64_Sym* symbol_table = nullptr; |
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.
I am not sure if this will work on windows
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 particular implementation definitely wouldn't work on Windows, and is in the #ifdef
for the linux-specific code. Currently, the Windows implementation returns an empty list.
My goal was partly to avoid needing to alter an existing |
As an alternative implementation for the same end-user functionality, what would you think about a lowering pass that is applied just before Edit: Thinking on it, I like this approach more and more. While inspecting the symbol table would only let us determine the function names, generating the metadata in a |
the only main issue about generate approach is that it may not be comprehensive if we in the end link multiple exports together, which is a less frequent usecase as in static library case(where we append the symbol prefix). I think this is more of an optional feature rather than mandatory feature, so we should avoid exposing it as things like |
That's a good point, though I believe we'll have the correct behavior by default. For both the (As an extension, we could indicate which functions are present within the static and/or imported modules, but that would be a later extension.)
I think we want to expose it to the user, for use debugging. If a user receives an error message with suggested typo corrections, or stating that there no similarly named functions, the obvious next step is to look at what functions actually are available. Since |
I've reverted the implementation of this PR, but kept the same unit tests. The platform-dependent implementation is entirely removed, replaced with a TIR lowering pass. |
Prior to this commit, the `LLVMModuleNode` provided a `"get_func_names"` function that would return the names of functions available within the result of `tvm.build`. However, this utility was not preserved across a round-trip through `mod.export_library` and `tvm.runtime.load_module`. This commit adds a similar `"__get_func_names"` function to a `DSOLibrary`, which returns the symbols available for use in the library. This is exposed in the `Module.keys()` method, mimicking the interface that a Python user would expect. In addition, this is used to improve the error message shown when a library does not contain the requested function. The `difflib` module (from Python's stdlib) is used to find functions that are contained in the module, and have similar names to the one requested.
934bb63
to
74456cf
Compare
74456cf
to
41a0662
Compare
Looks like the re-implemented version has trouble with RPC devices and with microtvm runtime. In order to compile the metadata function, I needed to add the ability to return a There is no straightforward way to perform the equivalent of I think the ability to query a module's contents is too useful to set aside entirely, but this PR is going on the back burner for me until I can find a better way to implement it. |
Making a few notes here for later. The key limitation is that a
Idea for a design:
Steps 1/2/3 would all be in C++, and would be straightforward to implement with the existing APIs. Step 4 would allow a PrimFunc to return a non-owning |
Prior to this commit, the
LLVMModuleNode
provided a"get_func_names"
function that would return the names of functions available within the result oftvm.build
. However, this utility was not preserved across a round-trip throughmod.export_library
andtvm.runtime.load_module
.This commit adds a similar
"__get_func_names"
function to aDSOLibrary
, which returns the symbols available for use in the library. This is exposed in theModule.keys()
method, mimicking the interface that a Python user would expect.In addition, this is used to improve the error message shown when a library does not contain the requested function. The
difflib
module (from Python's stdlib) is used to find functions that are contained in the module, and have similar names to the one requested.