Skip to content

Commit 398ce66

Browse files
larryliu0820facebook-github-bot
authored andcommitted
Let custom_ops_aot_lib use portable_lib (#4024)
Summary: Currently on Mac we can't import portable_lib then import sdpa_with_kv_cache. The reason is that they both statically link `executorch_no_prim_ops.a` and when they are both dlopen'd in python they are holding 2 copies of the static variables in `executorch_no_prim_ops.a` such as [initialized](https://github.com/pytorch/executorch/blob/main/runtime/platform/default/posix.cpp#L70). Pull Request resolved: #4024 Test Plan: On Mac, make sure the pybind custom ops is able to import. ``` from executorch.extension.pybindings import portable_lib from executorch.examples.models.llama2.custom_ops import sdpa_with_kv_cache portable_lib._get_operator_names() > ... > llama::sdpa_with_kv_cache.out ``` Reviewed By: dbort Differential Revision: D58883985 Pulled By: larryliu0820 fbshipit-source-id: 0a42d90a53a90da83f8b57504844cbf67688d8f8
1 parent 78688b7 commit 398ce66

File tree

2 files changed

+29
-32
lines changed

2 files changed

+29
-32
lines changed

CMakeLists.txt

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -519,13 +519,6 @@ target_link_options_shared_lib(executorch)
519519
#
520520
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/kernels/portable)
521521

522-
if(EXECUTORCH_BUILD_KERNELS_CUSTOM)
523-
# TODO: move all custom kernels to ${CMAKE_CURRENT_SOURCE_DIR}/kernels/custom
524-
add_subdirectory(
525-
${CMAKE_CURRENT_SOURCE_DIR}/examples/models/llama2/custom_ops
526-
)
527-
endif()
528-
529522
if(EXECUTORCH_BUILD_KERNELS_OPTIMIZED)
530523
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/kernels/optimized)
531524
endif()
@@ -688,17 +681,6 @@ if(EXECUTORCH_BUILD_PYBIND)
688681
list(APPEND _dep_libs quantized_kernels quantized_ops_lib)
689682
endif()
690683

691-
# TODO(larryliu): Fix macOS 2 dylibs having 2 sets of static variables issue
692-
if(EXECUTORCH_BUILD_KERNELS_CUSTOM_AOT AND NOT APPLE)
693-
list(APPEND _dep_libs custom_ops_aot_lib)
694-
endif()
695-
# TODO(laryliu): Fix linux duplicate registation problem. In GH CI worker
696-
# libcustom_ops.a doesn't dedup with the one indirectly linked from
697-
# libcustom_ops_aot_lib.a
698-
if(EXECUTORCH_BUILD_KERNELS_CUSTOM AND APPLE)
699-
target_link_options_shared_lib(custom_ops)
700-
list(APPEND _dep_libs custom_ops)
701-
endif()
702684
# compile options for pybind
703685
set(_pybind_compile_options
704686
-Wno-deprecated-declarations
@@ -723,7 +705,7 @@ if(EXECUTORCH_BUILD_PYBIND)
723705
target_link_libraries(util PRIVATE torch c10 executorch)
724706

725707
# pybind portable_lib
726-
pybind11_add_module(portable_lib extension/pybindings/pybindings.cpp)
708+
pybind11_add_module(portable_lib SHARED extension/pybindings/pybindings.cpp)
727709
# The actual output file needs a leading underscore so it can coexist with
728710
# portable_lib.py in the same python package.
729711
set_target_properties(portable_lib PROPERTIES OUTPUT_NAME "_portable_lib")
@@ -732,7 +714,7 @@ if(EXECUTORCH_BUILD_PYBIND)
732714
)
733715
target_include_directories(portable_lib PRIVATE ${TORCH_INCLUDE_DIRS})
734716
target_compile_options(portable_lib PUBLIC ${_pybind_compile_options})
735-
target_link_libraries(portable_lib PUBLIC ${_dep_libs})
717+
target_link_libraries(portable_lib PRIVATE ${_dep_libs})
736718
if(APPLE)
737719
# pip wheels will need to be able to find the torch libraries. On Linux, the
738720
# .so has non-absolute dependencies on libs like "libtorch.so" without
@@ -758,5 +740,12 @@ if(EXECUTORCH_BUILD_PYBIND)
758740
)
759741
endif()
760742

743+
if(EXECUTORCH_BUILD_KERNELS_CUSTOM)
744+
# TODO: move all custom kernels to ${CMAKE_CURRENT_SOURCE_DIR}/kernels/custom
745+
add_subdirectory(
746+
${CMAKE_CURRENT_SOURCE_DIR}/examples/models/llama2/custom_ops
747+
)
748+
endif()
749+
761750
# Print all summary
762751
executorch_print_configuration_summary()

examples/models/llama2/custom_ops/CMakeLists.txt

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,7 @@ include(${EXECUTORCH_SRCS_FILE})
4040
set(_common_include_directories ${EXECUTORCH_ROOT}/..)
4141

4242
# Custom op libraries
43-
set(custom_ops_libs)
44-
if(APPLE)
45-
list(APPEND executorch_no_prim_ops_shared)
46-
else()
47-
list(APPEND executorch_no_prim_ops)
48-
endif()
49-
list(APPEND custom_ops_libs pthreadpool)
43+
set(custom_ops_libs pthreadpool)
5044
list(APPEND custom_ops_libs cpuinfo)
5145
list(APPEND custom_ops_libs cpublas)
5246
list(APPEND custom_ops_libs eigen_blas)
@@ -72,7 +66,9 @@ target_include_directories(custom_ops PUBLIC "${_common_include_directories}")
7266
target_include_directories(
7367
custom_ops PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/../../../../include"
7468
)
75-
target_link_libraries(custom_ops PUBLIC ${custom_ops_libs})
69+
target_link_libraries(
70+
custom_ops PUBLIC ${custom_ops_libs} executorch_no_prim_ops
71+
)
7672

7773
target_compile_options(
7874
custom_ops PUBLIC ${_common_compile_options} -DET_USE_THREADPOOL
@@ -84,7 +80,8 @@ if(EXECUTORCH_BUILD_KERNELS_CUSTOM_AOT)
8480
# Add a AOT library
8581
find_package(Torch CONFIG REQUIRED)
8682
add_library(
87-
custom_ops_aot_lib SHARED ${CMAKE_CURRENT_SOURCE_DIR}/op_sdpa_aot.cpp
83+
custom_ops_aot_lib SHARED ${_custom_ops__srcs}
84+
${CMAKE_CURRENT_SOURCE_DIR}/op_sdpa_aot.cpp
8885
)
8986
target_include_directories(
9087
custom_ops_aot_lib PUBLIC "${_common_include_directories}"
@@ -93,10 +90,21 @@ if(EXECUTORCH_BUILD_KERNELS_CUSTOM_AOT)
9390
custom_ops_aot_lib
9491
PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/../../../../include"
9592
)
96-
target_link_libraries(custom_ops_aot_lib PUBLIC custom_ops torch)
93+
if(TARGET portable_lib)
94+
# If we have portable_lib built, custom_ops_aot_lib gives the ability to use
95+
# the ops in PyTorch and ExecuTorch through pybind
96+
target_link_libraries(custom_ops_aot_lib PUBLIC portable_lib)
97+
else()
98+
# If no portable_lib, custom_ops_aot_lib still gives the ability to use the
99+
# ops in PyTorch
100+
target_link_libraries(custom_ops_aot_lib PUBLIC executorch_no_prim_ops)
101+
endif()
102+
103+
target_link_libraries(custom_ops_aot_lib PUBLIC cpublas torch)
97104
target_compile_options(
98-
custom_ops_aot_lib PUBLIC -Wno-deprecated-declarations -fPIC -frtti
99-
-fexceptions
105+
custom_ops_aot_lib
106+
PUBLIC -Wno-deprecated-declarations -fPIC -frtti -fexceptions
107+
${_common_compile_options} -DET_USE_THREADPOOL
100108
)
101109

102110
install(TARGETS custom_ops_aot_lib DESTINATION lib)

0 commit comments

Comments
 (0)