From f1be92faf0a4085921116594d3b2457e3a383043 Mon Sep 17 00:00:00 2001 From: Edward Chen <18449977+edgchen1@users.noreply.github.com> Date: Thu, 3 Oct 2024 14:17:15 -0700 Subject: [PATCH] Patch fp16 to fix Xcode 16 builds with XNNPACK EP targeting x86_64. (#22294) --- .github/workflows/mac.yml | 50 ++++++++++++++++++- cmake/external/xnnpack.cmake | 33 +++++++++++- cmake/patches/fp16/readme.md | 11 ++++ ...remove_math_h_dependency_from_fp16_h.patch | 29 +++++++++++ 4 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 cmake/patches/fp16/readme.md create mode 100644 cmake/patches/fp16/remove_math_h_dependency_from_fp16_h.patch diff --git a/.github/workflows/mac.yml b/.github/workflows/mac.yml index aecc05c91d736..63b3e0e6e6e36 100644 --- a/.github/workflows/mac.yml +++ b/.github/workflows/mac.yml @@ -19,7 +19,7 @@ env: python_version: 3.11 jobs: - ARM64: + ARM64-Xcode16: runs-on: macos-14 env: @@ -60,6 +60,54 @@ jobs: --use_xnnpack \ --use_binskim_compliant_compile_flags + ARM64-Xcode16-targeting-iphonesimulator-x86_64: + runs-on: macos-14 + + env: + xcode_version: 16 + + timeout-minutes: 60 + + steps: + - uses: actions/setup-python@v5 + with: + python-version: ${{ env.python_version }} + + - name: Verify ARM64 machine + shell: python + run: | + import platform + assert platform.machine() == "arm64", "This job expects to be run on an ARM64 machine." + + - name: Use Xcode ${{ env.xcode_version }} + shell: bash + run: | + XCODE_DEVELOPER_DIR="/Applications/Xcode_${{ env.xcode_version }}.app/Contents/Developer" + sudo xcode-select --switch "${XCODE_DEVELOPER_DIR}" + + - uses: actions/checkout@v4 + + # Note: Setting onnxruntime_BUILD_UNIT_TESTS=OFF as a workaround for + # https://github.com/microsoft/onnxruntime/issues/22245. + - name: Build + shell: bash + run: | + python ./tools/ci_build/build.py \ + --build_dir ./build \ + --update \ + --build --parallel \ + --skip_tests \ + --build_apple_framework \ + --use_xcode \ + --use_coreml \ + --use_xnnpack \ + --use_binskim_compliant_compile_flags \ + --ios \ + --apple_deploy_target=13.0 \ + --apple_sysroot=iphonesimulator \ + --osx_arch=x86_64 \ + --cmake_extra_defines=onnxruntime_BUILD_UNIT_TESTS=OFF + Vcpkg: runs-on: macos-13 steps: diff --git a/cmake/external/xnnpack.cmake b/cmake/external/xnnpack.cmake index 26d56b1cbf45c..3298c078b592a 100644 --- a/cmake/external/xnnpack.cmake +++ b/cmake/external/xnnpack.cmake @@ -21,7 +21,38 @@ endif() FetchContent_Declare(psimd URL ${DEP_URL_psimd} URL_HASH SHA1=${DEP_SHA1_psimd}) onnxruntime_fetchcontent_makeavailable(psimd) set(PSIMD_SOURCE_DIR ${psimd_SOURCE_DIR}) -FetchContent_Declare(fp16 URL ${DEP_URL_fp16} URL_HASH SHA1=${DEP_SHA1_fp16}) + +block(PROPAGATE fp16_PATCH_COMMAND) + # only apply fp16 patch for Apple x86_64 targets + + if(APPLE) + if(NOT "${CMAKE_OSX_ARCHITECTURES}" STREQUAL "") + if ("x86_64" IN_LIST CMAKE_OSX_ARCHITECTURES) + set(fp16_PATCH_REQUIRED 1) + endif() + else() + # CMAKE_OSX_ARCHITECTURES unspecified, check host + if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64") + set(fp16_PATCH_REQUIRED 1) + endif() + endif() + endif() + + if(fp16_PATCH_REQUIRED) + message(STATUS "Applying fp16 patch.") + set(fp16_PATCH_FILE ${PROJECT_SOURCE_DIR}/patches/fp16/remove_math_h_dependency_from_fp16_h.patch) + set(fp16_PATCH_COMMAND ${Patch_EXECUTABLE} --binary --ignore-whitespace -p1 < ${fp16_PATCH_FILE}) + else() + set(fp16_PATCH_COMMAND "") + endif() +endblock() + +FetchContent_Declare( + fp16 + URL ${DEP_URL_fp16} + URL_HASH SHA1=${DEP_SHA1_fp16} + PATCH_COMMAND ${fp16_PATCH_COMMAND} + ) onnxruntime_fetchcontent_makeavailable(fp16) # pthreadpool depends on fxdiv diff --git a/cmake/patches/fp16/readme.md b/cmake/patches/fp16/readme.md new file mode 100644 index 0000000000000..a39c44e4807b6 --- /dev/null +++ b/cmake/patches/fp16/readme.md @@ -0,0 +1,11 @@ +# remove_math_h_dependency_from_fp16_h.patch + +Remove dependency on math.h (with fabsf()) to work around Xcode 16 build error for iphonesimulator x86_64 target: + +/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/usr/include/math.h:614:27: error: + _Float16 is not supported on this target + 614 | extern _Float16 __fabsf16(_Float16) __API_AVAILABLE(macos(15.0), ios(18.0), watchos(11.0), tvos(18.0)); + | + +This patch was adapted from this PR: https://github.com/Maratyszcza/FP16/pull/32 +See also: https://github.com/google/XNNPACK/issues/6989 diff --git a/cmake/patches/fp16/remove_math_h_dependency_from_fp16_h.patch b/cmake/patches/fp16/remove_math_h_dependency_from_fp16_h.patch new file mode 100644 index 0000000000000..6f700f8ef4aba --- /dev/null +++ b/cmake/patches/fp16/remove_math_h_dependency_from_fp16_h.patch @@ -0,0 +1,29 @@ +diff --git a/include/fp16/fp16.h b/include/fp16/fp16.h +index 2b61fff..1947805 100644 +--- a/include/fp16/fp16.h ++++ b/include/fp16/fp16.h +@@ -4,10 +4,8 @@ + + #if defined(__cplusplus) && (__cplusplus >= 201103L) + #include +- #include + #elif !defined(__OPENCL_VERSION__) + #include +- #include + #endif + + #ifdef _MSC_VER +@@ -228,9 +226,11 @@ static inline uint16_t fp16_ieee_from_fp32_value(float f) { + const float scale_to_inf = fp32_from_bits(UINT32_C(0x77800000)); + const float scale_to_zero = fp32_from_bits(UINT32_C(0x08800000)); + #endif +- float base = (fabsf(f) * scale_to_inf) * scale_to_zero; +- + const uint32_t w = fp32_to_bits(f); ++ const float abs_f = fp32_from_bits(w & UINT32_C(0x7FFFFFFF)); ++ ++ float base = (abs_f * scale_to_inf) * scale_to_zero; ++ + const uint32_t shl1_w = w + w; + const uint32_t sign = w & UINT32_C(0x80000000); + uint32_t bias = shl1_w & UINT32_C(0xFF000000);