Skip to content

Fix libflatccrt race #10918

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

Merged
merged 1 commit into from
May 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions .ci/scripts/build-qnn-sdk.sh
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,12 @@ set -o xtrace

build_qnn_backend() {
echo "Start building qnn backend."
export ANDROID_NDK_ROOT=/opt/ndk
export QNN_SDK_ROOT=/tmp/qnn/2.28.0.241029
export ANDROID_NDK_ROOT=${ANDROID_NDK_ROOT:-/opt/ndk}
export QNN_SDK_ROOT=${QNN_SDK_ROOT:-/tmp/qnn/2.28.0.241029}
export EXECUTORCH_ROOT="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/../.." && pwd)"

# Workaround to avoid issues around missing flatccrt library (depending on the
# number of jobs used), see issue #7300:
# Build twice (second time with `--no_clean`) to make sure libflatccrt.a is
# available.
# TODO: Remove this workaround once the underlying issue is fixed.
bash backends/qualcomm/scripts/build.sh --skip_aarch64 --job_number 2 --release || \
bash backends/qualcomm/scripts/build.sh --skip_aarch64 --job_number 2 --release --no_clean
Comment on lines -23 to -24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change related?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, see the comment right above — this was the way the race was mitigated before in #7570

parallelism=$(( $(nproc) - 1 ))
bash backends/qualcomm/scripts/build.sh --skip_aarch64 --job_number ${parallelism} --release
}

set_up_aot() {
Expand Down
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,7 @@ if(EXECUTORCH_BUILD_PYBIND)
${TORCH_PYTHON_LIBRARY}
bundled_program
etdump
flatccrt
executorch
extension_data_loader
util
Expand Down
8 changes: 1 addition & 7 deletions examples/apple/mps/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,6 @@ if(NOT CMAKE_TOOLCHAIN_FILE MATCHES ".*(iOS|ios\.toolchain)\.cmake$")
list(TRANSFORM _mps_executor_runner__srcs PREPEND "${EXECUTORCH_ROOT}/")
add_executable(mps_executor_runner ${_mps_executor_runner__srcs})

if(CMAKE_BUILD_TYPE MATCHES "Debug")
set(FLATCC_LIB flatccrt_d)
else()
set(FLATCC_LIB flatccrt)
endif()

Comment on lines -100 to -105
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you confirmed that debug build will also build flatccrt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it seems to have both libs

if(CMAKE_BUILD_TYPE MATCHES "Debug")
target_link_options(mps_executor_runner PUBLIC -fsanitize=undefined)
endif()
Expand All @@ -113,7 +107,7 @@ if(NOT CMAKE_TOOLCHAIN_FILE MATCHES ".*(iOS|ios\.toolchain)\.cmake$")
executorch
gflags
etdump
${FLATCC_LIB}
flatccrt
mpsdelegate
mps_portable_ops_lib
${mps_executor_runner_libs}
Expand Down
14 changes: 4 additions & 10 deletions examples/arm/executor_runner/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -583,22 +583,16 @@ if(EXECUTORCH_ENABLE_EVENT_TRACER)
"${ET_BUILD_DIR_PATH}/lib/libetdump.a"
)

if(CMAKE_BUILD_TYPE MATCHES "Debug")
set(FLATCCRT_LIB flatccrt_d)
else()
set(FLATCCRT_LIB flatccrt)
endif()

add_library(${FLATCCRT_LIB} STATIC IMPORTED)
add_library(flatccrt STATIC IMPORTED)
set_property(
TARGET ${FLATCCRT_LIB}
TARGET flatccrt
PROPERTY IMPORTED_LOCATION
"${ET_BUILD_DIR_PATH}/lib/lib${FLATCCRT_LIB}.a"
"${ET_BUILD_DIR_PATH}/lib/libflatccrt.a"
)

list(APPEND arm_executor_runner_link
etdump
${FLATCCRT_LIB}
flatccrt
)
endif()

Expand Down
2 changes: 1 addition & 1 deletion examples/qualcomm/executor_runner/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ target_include_directories(
)
target_link_libraries(
qnn_executor_runner qnn_executorch_backend full_portable_ops_lib etdump
${FLATCCRT_LIB} gflags
flatccrt gflags
)
set_target_properties(
qnn_executor_runner PROPERTIES LINK_FLAGS "-Wl,-rpath='$ORIGIN'"
Expand Down
6 changes: 6 additions & 0 deletions third-party/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ ExternalProject_Add(
-DCMAKE_OSX_DEPLOYMENT_TARGET:STRING=${CMAKE_OSX_DEPLOYMENT_TARGET}
BUILD_BYPRODUCTS <INSTALL_DIR>/bin/flatcc
)
file(REMOVE_RECURSE ${PROJECT_SOURCE_DIR}/third-party/flatcc/lib)
ExternalProject_Get_Property(flatcc_external_project INSTALL_DIR)
add_executable(flatcc_cli IMPORTED GLOBAL)
add_dependencies(flatcc_cli flatcc_external_project)
Expand All @@ -83,6 +84,11 @@ set(FLATCC_REFLECTION OFF CACHE BOOL "")
set(FLATCC_DEBUG_CLANG_SANITIZE OFF CACHE BOOL "")
set(FLATCC_INSTALL OFF CACHE BOOL "")
add_subdirectory(flatcc)
# Unfortunately flatcc writes libs directly in to the source tree [1]. So to
# ensure the target lib is created last, force flatcc_cli to build first.
#
# [1] https://github.com/dvidelabs/flatcc/blob/896db54787e8b730a6be482c69324751f3f5f117/CMakeLists.txt#L168
add_dependencies(flatccrt flatcc_cli)
# Fix for "relocation R_X86_64_32 against `.rodata' can not be used when making
# a shared object; recompile with -fPIC" when building on some x86 linux
# systems.
Expand Down
12 changes: 5 additions & 7 deletions tools/cmake/executorch-config.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,12 @@ set(EXECUTORCH_FOUND ON)

target_link_libraries(executorch INTERFACE executorch_core)

if(CMAKE_BUILD_TYPE MATCHES "Debug")
set(FLATCCRT_LIB flatccrt_d)
else()
set(FLATCCRT_LIB flatccrt)
endif()

set(lib_list
flatccrt
etdump
bundled_program
extension_data_loader
extension_flat_tensor
${FLATCCRT_LIB}
coreml_util
coreml_inmemoryfs
coremldelegate
Expand Down Expand Up @@ -154,6 +148,10 @@ if(TARGET coremldelegate)
)
endif()

if(TARGET etdump)
set_target_properties(etdump PROPERTIES INTERFACE_LINK_LIBRARIES "flatccrt;executorch")
endif()

if(TARGET optimized_native_cpu_ops_lib)
if(TARGET optimized_portable_kernels)
set(_maybe_optimized_portable_kernels_lib optimized_portable_kernels)
Expand Down
Loading