Skip to content

Commit

Permalink
Fix xnnpack pybind (#2802)
Browse files Browse the repository at this point in the history
Summary:
Fixing XNNPACK Pybind.

Currently pybindings for xnnpack fail due to uninitialized error. We notice the following:
```
(gdb) info symbol xnn_initialize
xnn_initialize in section .text of /home/maxren/.conda/envs/executorch/lib/python3.10/site-packages/torch/lib/libtorch_cpu.so
(gdb) info symbol xnn_create_subgraph
xnn_create_subgraph in section .text of /home/maxren/.conda/envs/executorch/lib/python3.10/site-packages/executorch/extension/pybindings/portable_lib.cpython-310-x86_64-linux-gnu.so
```

That is xnn_initialize symbole comes from libtorch_cpu, but when we call xnn_create_subgraph we call it from another library. It seems like explicitly adding XNNPACK to the portable_libs pybind fixes this problem.

Additionaly, we fix some fPIC issues when building XNNPACK.

Pull Request resolved: #2802

Reviewed By: larryliu0820

Differential Revision: D55646601

Pulled By: mcr229

fbshipit-source-id: cf18de3246c9fdde4c08326c400d19cd407720e1
  • Loading branch information
mcr229 authored and facebook-github-bot committed Apr 2, 2024
1 parent 8ab6daf commit 26d7bcc
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 5 deletions.
1 change: 1 addition & 0 deletions .github/workflows/_unittest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ jobs:
# Setup MacOS dependencies as there is no Docker support on MacOS atm
PYTHON_EXECUTABLE=python \
EXECUTORCH_BUILD_PYBIND=ON \
EXECUTORCH_BUILD_XNNPACK=ON \
.ci/scripts/setup-linux.sh "${BUILD_TOOL}"
# Run pytest with coverage
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/pull.yml
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ jobs:
# build module for executorch.extension.pybindings.portable_lib
BUILD_TOOL=${{ matrix.build-tool }}
PYTHON_EXECUTABLE=python \
EXECUTORCH_BUILD_XNNPACK=ON \
EXECUTORCH_BUILD_PYBIND=ON \
bash .ci/scripts/setup-linux.sh "${BUILD_TOOL}"
Expand Down
4 changes: 3 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,9 @@ if(EXECUTORCH_BUILD_PYBIND)
endif()

if(EXECUTORCH_BUILD_XNNPACK)
set(PYBIND_LINK_XNNPACK "xnnpack_backend")
# need to explicitly specify XNNPACK here
# otherwise uses XNNPACK symbols from libtorch_cpu
set(PYBIND_LINK_XNNPACK xnnpack_backend XNNPACK)
endif()

# find pytorch lib, to allow pybind to take at::Tensor as input/output
Expand Down
7 changes: 3 additions & 4 deletions backends/xnnpack/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ if(NOT PYTHON_EXECUTABLE)
endif()

set(_common_include_directories ${EXECUTORCH_ROOT}/..)
set(_common_compile_options -Wno-deprecated-declarations)
set(_common_compile_options -Wno-deprecated-declarations -fPIC)

set(_xnnpack_schema__include_dir "${CMAKE_BINARY_DIR}/schema/include")
# Paths to headers generated from the .fbs files.
Expand Down Expand Up @@ -72,7 +72,7 @@ target_include_directories(
xnnpack_schema INTERFACE ${_xnnpack_schema__include_dir}
${EXECUTORCH_ROOT}/third-party/flatbuffers/include)

set(xnnpack_third_party)
set(xnnpack_third_party pthreadpool cpuinfo)

include(cmake/Dependencies.cmake)

Expand Down Expand Up @@ -105,8 +105,7 @@ if(NOT CMAKE_TOOLCHAIN_FILE MATCHES ".*iOS\.cmake$")
list(TRANSFORM _xnn_executor_runner__srcs PREPEND "${EXECUTORCH_ROOT}/")
add_executable(xnn_executor_runner ${_xnn_executor_runner__srcs})
target_link_libraries(xnn_executor_runner
xnnpack_backend gflags portable_ops_lib
pthreadpool cpuinfo)
xnnpack_backend gflags portable_ops_lib)
target_compile_options(xnn_executor_runner PUBLIC ${_common_compile_options})
endif()

Expand Down
10 changes: 10 additions & 0 deletions backends/xnnpack/cmake/Dependencies.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
set(THIRD_PARTY_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/third-party")

# --- XNNPACK

# Setting this global PIC flag for all XNNPACK targets.
# This is needed for Object libraries within XNNPACK which must
# be PIC to successfully link this static libXNNPACK
set(ORIGINAL_CMAKE_POSITION_INDEPENDENT_CODE_FLAG ${CMAKE_POSITION_INDEPENDENT_CODE})
set(CMAKE_POSITION_INDEPENDENT_CODE ON)

set(XNNPACK_SOURCE_DIR "${THIRD_PARTY_ROOT}/XNNPACK")
set(XNNPACK_INCLUDE_DIR "${XNNPACK_SOURCE_DIR}/include")
set(XNNPACK_LIBRARY_TYPE "static" CACHE STRING "")
Expand All @@ -18,3 +25,6 @@ set(XNNPACK_ENABLE_AVXVNNI OFF CACHE BOOL "")
add_subdirectory("${XNNPACK_SOURCE_DIR}")
include_directories(SYSTEM ${XNNPACK_INCLUDE_DIR})
list(APPEND xnnpack_third_party XNNPACK)

# Revert PIC Flag to what it originally was
set(CMAKE_POSITION_INDEPENDENT_CODE ${ORIGINAL_CMAKE_POSITION_INDEPENDENT_CODE_FLAG})
2 changes: 2 additions & 0 deletions pytest.ini
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ addopts =
kernels/test/test_case_gen.py
# backends/arm
backends/arm/test
# backends/xnnpack
backends/xnnpack/test/models
# test
test/end2end/test_end2end.py

Expand Down

0 comments on commit 26d7bcc

Please sign in to comment.