Add executorch_no_prim_ops target (pytorch#2934)

Summary: Pull Request resolved: pytorch#2934 Currently `libexecutorch.a` always contain prim ops. This becomes a problem when a binary contains 2 "versions" of `libexecutorch.a`, causing a double registration of the prim ops. For example, `libA.so` depends on `libexecutorch.a` and a binary `B` depends on both `libA.so` and `libexecutorch.a`. Since both `libexecutorch.a` and `libA.so` contains prim ops, they will be registered twice. In this PR I created another library `executorch_no_prim_ops` for `libA.so` to depend on. Reviewed By: cccclai, kirklandsign Differential Revision: D55907752 fbshipit-source-id: 755a9b8d5f6f7cf44d011b83bfdc18be6da1aa05 (cherry picked from commit d309e9d)
kirklandsign · Apr 11, 2024 · 763ccb9 · 763ccb9
1 parent 16c3afc
commit 763ccb9
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 46 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -352,23 +352,27 @@ add_subdirectory(schema)
 # Only contains primitive operators; does not contain portable kernels or other
 # full operators. Does not contain any backends.
 #
-
-add_library(executorch ${_executorch__srcs})
-target_link_libraries(executorch PRIVATE program_schema)
-target_link_options_shared_lib(executorch)
+add_library(executorch_no_prim_ops ${_executorch_no_prim_ops__srcs})
+target_link_libraries(executorch_no_prim_ops PRIVATE program_schema)
 # Check if dl exists for this toolchain and only then link it.
 find_library(DL_LIBRARY_EXISTS NAMES dl)
 # Check if the library was found
 if(DL_LIBRARY_EXISTS)
-  target_link_libraries(executorch PRIVATE dl) # For dladdr()
+  target_link_libraries(executorch_no_prim_ops PRIVATE dl) # For dladdr()
 endif()
-target_include_directories(executorch PUBLIC ${_common_include_directories})
-target_compile_options(executorch PUBLIC ${_common_compile_options})
+target_include_directories(executorch_no_prim_ops PUBLIC ${_common_include_directories})
+target_compile_options(executorch_no_prim_ops PUBLIC ${_common_compile_options})
 if(MAX_KERNEL_NUM)
-  target_compile_definitions(executorch
+  target_compile_definitions(executorch_no_prim_ops
                              PRIVATE MAX_KERNEL_NUM=${MAX_KERNEL_NUM})
 endif()
 
+add_library(executorch ${_executorch__srcs})
+target_link_libraries(executorch PRIVATE executorch_no_prim_ops)
+target_include_directories(executorch PUBLIC ${_common_include_directories})
+target_compile_options(executorch PUBLIC ${_common_compile_options})
+target_link_options_shared_lib(executorch)
+
 #
 # portable_ops_lib: A library to register core ATen ops using portable kernels,
 # see kernels/portable/CMakeLists.txt.
@@ -406,7 +410,7 @@ endif()
 # Install `executorch` library as well as `executorch-config.cmake` under
 # ${CMAKE_INSTALL_PREFIX}/
 install(
-  TARGETS executorch
+  TARGETS executorch executorch_no_prim_ops
   DESTINATION lib
   INCLUDES
   DESTINATION ${_common_include_directories})

diff --git a/build/cmake_deps.toml b/build/cmake_deps.toml
@@ -19,6 +19,18 @@ excludes = [
 buck_targets = [
   "//runtime/executor:program",
 ]
+deps = [
+  "executorch_no_prim_ops",
+]
+filters = [
+  ".cpp$",
+]
+
+
+[targets.executorch_no_prim_ops]
+buck_targets = [
+  "//runtime/executor:program_no_prim_ops",
+]
 deps = [
   "program_schema",
 ]
@@ -43,6 +55,7 @@ excludes = [
 ]
 deps = [
   "executorch",
+  "executorch_no_prim_ops",
 ]
 
 [targets.optimized_kernels]
@@ -59,6 +72,7 @@ excludes = [
 ]
 deps = [
   "executorch",
+  "executorch_no_prim_ops",
   "portable_kernels",
 ]
 
@@ -76,6 +90,7 @@ excludes = [
 ]
 deps = [
   "executorch",
+  "executorch_no_prim_ops",
   "portable_kernels",
 ]
 
@@ -97,6 +112,7 @@ filters = [
 excludes = [
 ]
 deps = [
+  "executorch_no_prim_ops",
   "executorch",
 ]
 
@@ -113,6 +129,7 @@ filters = [
   ".cpp$",
 ]
 deps = [
+  "executorch_no_prim_ops",
   "executorch",
 ]
 
@@ -125,6 +142,7 @@ filters = [
 ]
 deps = [
   "executorch",
+  "executorch_no_prim_ops",
   "extension_data_loader",
 ]
 
@@ -137,6 +155,7 @@ filters = [
 ]
 deps = [
   "executorch",
+  "executorch_no_prim_ops",
 ]
 
 # ---------------------------------- extension end ----------------------------------
@@ -154,6 +173,7 @@ excludes = [
 ]
 deps = [
   "executorch",
+  "executorch_no_prim_ops",
   "portable_kernels",
   "quantized_kernels",
 ]
@@ -169,6 +189,7 @@ excludes = [
   "^codegen",
 ]
 deps = [
+  "executorch_no_prim_ops",
   "executorch",
 ]
 # ---------------------------------- binary end ----------------------------------
@@ -185,6 +206,7 @@ excludes = [
 ]
 deps = [
   "executorch",
+  "executorch_no_prim_ops",
   "portable_kernels",
 ]
 
@@ -197,6 +219,7 @@ filters = [
 ]
 deps = [
   "executorch",
+  "executorch_no_prim_ops",
 ]
 
 [targets.mps_schema]
@@ -222,6 +245,7 @@ excludes = [
 ]
 deps = [
   "executorch",
+  "executorch_no_prim_ops",
   "xnnpack_backend",
   "portable_kernels",
 ]
@@ -235,6 +259,7 @@ filters = [
 ]
 deps = [
   "executorch",
+  "executorch_no_prim_ops",
 ]
 
 [targets.xnnpack_dynamic_quant_utils]
@@ -275,6 +300,7 @@ excludes = [
 ]
 deps = [
   "executorch",
+  "executorch_no_prim_ops",
   "optimized_kernels",
   "xnnpack_backend",
 ]
@@ -292,6 +318,7 @@ excludes = [
 deps = [
   "custom_ops",
   "executorch",
+  "executorch_no_prim_ops",
   "extension_data_loader",
   "extension_module",
   "portable_kernels",

diff --git a/build/executorch-config.cmake b/build/executorch-config.cmake
@@ -13,27 +13,20 @@
 cmake_minimum_required(VERSION 3.19)
 
 set(_root "${CMAKE_CURRENT_LIST_DIR}/../..")
-add_library(executorch STATIC IMPORTED)
-find_library(
-    EXECUTORCH_LIBRARY_PATH executorch
-    HINTS "${_root}"
-    CMAKE_FIND_ROOT_PATH_BOTH
-)
-set_target_properties(
-    executorch PROPERTIES IMPORTED_LOCATION "${EXECUTORCH_LIBRARY_PATH}"
-)
-target_include_directories(executorch INTERFACE ${_root})
+set(required_lib_list executorch executorch_no_prim_ops portable_kernels)
+foreach(lib ${required_lib_list})
+    set(lib_var "LIB_${lib}")
+    add_library(${lib} STATIC IMPORTED)
+    find_library(
+        ${lib_var} ${lib} HINTS "${_root}" CMAKE_FIND_ROOT_PATH_BOTH
+    )
+    set_target_properties(
+        ${lib} PROPERTIES IMPORTED_LOCATION "${${lib_var}}"
+    )
+    target_include_directories(${lib} INTERFACE ${_root})
+endforeach()
 
-add_library(portable_kernels STATIC IMPORTED)
-find_library(
-    PORTABLE_KERNELS_PATH portable_kernels
-    HINTS "${_root}"
-    CMAKE_FIND_ROOT_PATH_BOTH
-)
-set_target_properties(
-    portable_kernels PROPERTIES IMPORTED_LOCATION "${PORTABLE_KERNELS_PATH}"
-)
-target_include_directories(portable_kernels INTERFACE ${_root})
+target_link_libraries(executorch INTERFACE executorch_no_prim_ops)
 
 if(CMAKE_BUILD_TYPE MATCHES "Debug")
     set(FLATCCRT_LIB flatccrt_d)

diff --git a/runtime/executor/targets.bzl b/runtime/executor/targets.bzl
@@ -44,44 +44,47 @@ def define_common_targets():
 
     for aten_mode in (True, False):
         aten_suffix = "_aten" if aten_mode else ""
-
         runtime.cxx_library(
             name = "program" + aten_suffix,
+            exported_deps = [
+                ":program_no_prim_ops" + aten_suffix,
+                "//executorch/kernels/prim_ops:prim_ops_registry" + aten_suffix,
+            ],
+            visibility = [
+                "//executorch/runtime/executor/...",
+                "@EXECUTORCH_CLIENTS",
+            ],
+        )
+
+        runtime.cxx_library(
+            name = "program_no_prim_ops" + aten_suffix,
             srcs = [
                 "method.cpp",
                 "method_meta.cpp",
                 "program.cpp",
                 "tensor_parser_exec_aten.cpp",
                 "tensor_parser{}.cpp".format(aten_suffix if aten_mode else "_portable"),
             ],
-            headers = [
-                "tensor_parser.h",
-            ],
             exported_headers = [
                 "method.h",
                 "method_meta.h",
                 "program.h",
+                "tensor_parser.h",
             ],
-            deps = [
-                "//executorch/kernels/prim_ops:prim_ops_registry" + aten_suffix,
+            preprocessor_flags = _program_preprocessor_flags(),
+            exported_deps = [
+                ":memory_manager",
                 "//executorch/runtime/backend:interface",
-                "//executorch/runtime/core/exec_aten/util:tensor_util" + aten_suffix,
                 "//executorch/runtime/core:core",
+                "//executorch/runtime/core:evalue" + aten_suffix,
+                "//executorch/runtime/core:event_tracer" + aten_suffix,
+                "//executorch/runtime/core/exec_aten:lib" + aten_suffix,
+                "//executorch/runtime/core/exec_aten/util:tensor_util" + aten_suffix,
                 "//executorch/runtime/kernel:kernel_runtime_context" + aten_suffix,
                 "//executorch/runtime/kernel:operator_registry",
                 "//executorch/runtime/platform:platform",
                 "//executorch/schema:extended_header",
                 "//executorch/schema:program",
-                ":memory_manager",
-            ],
-            preprocessor_flags = _program_preprocessor_flags(),
-            exported_deps = [
-                "//executorch/runtime/core/exec_aten:lib" + aten_suffix,
-                "//executorch/runtime/core:core",
-                "//executorch/runtime/core:evalue" + aten_suffix,
-                "//executorch/runtime/platform:platform",
-                "//executorch/runtime/core:event_tracer" + aten_suffix,
-                ":memory_manager",
             ],
             visibility = [
                 "//executorch/runtime/executor/...",