Skip to content

Add platform-specific configurations for iOS and macOS ARM64 to prevent AVX code compilation on x64 macOS - #6731

Closed
AkiSakurai wants to merge 2 commits into
pytorch:mainfrom
AkiSakurai:ios-avx2
Closed

Add platform-specific configurations for iOS and macOS ARM64 to prevent AVX code compilation on x64 macOS#6731
AkiSakurai wants to merge 2 commits into
pytorch:mainfrom
AkiSakurai:ios-avx2

Conversation

@AkiSakurai

Copy link
Copy Markdown
Contributor

…nt AVX code compilation on x64 macOS
- Updated `Utils.cmake` to set appropriate `target_platforms_arg` for iOS and macOS when building on ARM64.
- Added execution platforms `ios-arm64` and `macos-arm64` in `shim/BUCK` for specifying CPU and OS configurations.
- fixpytorch#6691
@pytorch-bot

pytorch-botBot commented Nov 8, 2024

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/6731

Note: Links to docs will display an error until the docs builds have been completed.

❗ 2 Active SEVs

There are 2 currently active SEVs. If your PR is affected, please view them below:

❌ 3 New Failures

As of commit 4c1d38f with merge base 39e5b91 (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-botfacebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 8, 2024
@JacobSzwejbka

Copy link
Copy Markdown
Contributor

@helunwencser@GregoryComer@larryliu0820 can you take a look

@larryliu0820

Copy link
Copy Markdown
Contributor

Thanks for fixing the issue! Can you please add a unit test / CI job for this? Or maybe list out the repro commands so someone else can add the test.

@zcbenz

Copy link
Copy Markdown

@AkiSakurai I think I have met the opposite issue with yours in #6839, do you have an idea how to make cross-compilation to x64 from arm64 work? I would be happy to test.

@zcbenz

Copy link
Copy Markdown

Actually I tried apply this patch and #6839 also gets fixed with it:

diff --git a/build/Utils.cmake b/build/Utils.cmake
index bdda2b220..d81a2c8ef 100644
--- a/build/Utils.cmake+++ b/build/Utils.cmake@@ -206,6 +206,8 @@ function(extract_sources sources_file)
if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
if(CMAKE_SYSTEM_PROCESSOR STREQUAL "aarch64")
set(target_platforms_arg "--target-platforms=shim//:macos-arm64")
+ elseif(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64")+ set(target_platforms_arg "--target-platforms=shim//:macos-x86_64")
endif()
endif()
diff --git a/shim/BUCK b/shim/BUCK
index c4b0c91a0..fa0d8464b 100644
--- a/shim/BUCK+++ b/shim/BUCK@@ -90,3 +90,11 @@ execution_platform(
use_windows_path_separators = False,
visibility = ["PUBLIC"],
)
++execution_platform(+ name = "macos-x86_64",+ cpu_configuration = "prelude//cpu:x86_64",+ os_configuration = "prelude//os:macos",+ use_windows_path_separators = False,+ visibility = ["PUBLIC"],+)

Maybe include it in this PR?

@AkiSakurai

Copy link
Copy Markdown
ContributorAuthor

Thanks for fixing the issue! Can you please add a unit test / CI job for this? Or maybe list out the repro commands so someone else can add the test.

To reproduce, run ./build/build_apple_frameworks.sh --coreml --custom --mps --optimized --portable --quantized --xnnpack on intel mac.

@shoumikhin

Copy link
Copy Markdown
Contributor

@AkiSakurai do we still need this change? Please make sure the tests pass.

@AkiSakurai

Copy link
Copy Markdown
ContributorAuthor

@AkiSakurai do we still need this change? Please make sure the tests pass.

No, it was fixed in #8346

@byjlwbyjlw closed this Mar 17, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

7 participants

@AkiSakurai@JacobSzwejbka@larryliu0820@zcbenz@shoumikhin@byjlw@facebook-github-bot