-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[mlir][python] handle more undefined symbols not covered by nanobind #153861
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
Conversation
@llvm/pr-subscribers-mlir Author: Maksim Levental (makslevental) ChangesIntroduced (but omitted from this CMake) in #151246. Full diff: https://github.com/llvm/llvm-project/pull/153861.diff 1 Files Affected:
diff --git a/mlir/cmake/modules/AddMLIRPython.cmake b/mlir/cmake/modules/AddMLIRPython.cmake
index c14e614ed7d92..2b883558d33c6 100644
--- a/mlir/cmake/modules/AddMLIRPython.cmake
+++ b/mlir/cmake/modules/AddMLIRPython.cmake
@@ -704,7 +704,12 @@ function(add_mlir_python_extension libname extname)
# NanobindAdaptors.h uses PyClassMethod_New to build `pure_subclass`es but nanobind
# doesn't declare this API as undefined in its linker flags. So we need to declare it as such
# for downstream users that do not do something like `-undefined dynamic_lookup`.
- target_link_options(${libname} PUBLIC "LINKER:-U,_PyClassMethod_New")
+ # Same for the rest.
+ target_link_options(${libname} PUBLIC
+ "LINKER:-U,_PyClassMethod_New"
+ "LINKER:-U,_PyCode_Addr2Location"
+ "LINKER:-U,_PyFrame_GetLasti"
+ )
endif()
endif()
|
Great! Just to check though: does this address the warnings that were being generated during recent builds, e.g.:
I think those are probably related to the same PR (or follow-up fixes). |
it doesn't but i can fix these in this PR too |
…d fix warning unused Introduced (but omitted from this CMake) in #151246.
d92f500
to
632c7a7
Compare
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/138/builds/17642 Here is the relevant piece of the build log for the reference
|
Introduced (but omitted from this CMake) in #151246.