-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Support CMAKE_OSX_ARCHITECTURES #8390
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
Conversation
| "COMMAND;DEPENDS;EXTRA_OUTPUTS;PARAMS;PLUGINS;TARGETS" | ||
| ) | ||
|
|
||
| ## "hash table" of extra outputs to extensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add 'hlpipe' here too? It should already be hooked up in the generator interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely. Didn't know it was missing!
| OUTPUT_VARIABLE merged_libs_targets) | ||
|
|
||
| find_program(LIPO lipo REQUIRED) | ||
| add_custom_command( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done, and with all the dependencies setup too!
| FUNCTION_NAME train_cost_model | ||
| TARGETS cmake | ||
| FEATURES[x86-64-osx] avx2 sse41 | ||
| FEATURES[arm-64-osx] arm_dot_prod-arm_fp16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these features available on all Apple Silicon?
3f4eb33 to
88701db
Compare
| function(_bundle_static_is_apple_libtool result item) | ||
| function(_bundle_static_is_apple_libtool result_var item) | ||
| _bundle_static_check_output(version_info "${item}" -V) | ||
| if (version_info MATCHES "Apple, Inc.") | ||
| set(result 1 PARENT_SCOPE) | ||
| else () | ||
| set(result 0 PARENT_SCOPE) | ||
| if (NOT version_info MATCHES "Apple, Inc.") | ||
| set(${result_var} 0 PARENT_SCOPE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a drive-by fix.
|
Topics for discussion at the dev meeting:
|
|
Finally, we have a clean build! The @derek-gerstmann -- I'd appreciate some help testing offline. |
|
Testing on my MacBook Pro M1 Max and everything appears to work! I agree, it'd be nice to find a testing strategy for both desktop and mobile. |
|
From dev meeting:
|
This is potentially a breaking change for some builds that aren't using add_halide_generator or vcpkg on Windows.
Previously _Halide_try_load_generators was unhygienic with respect to scope. Now it takes a proper argument.
8396298 to
8df371a
Compare
|
This is ready on my end. @steven-johnson -- we should coordinate adding testing to the buildbots. Should we wait to land this until we have that? |
yes, probably |
steven-johnson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there documentation/README for this anywhere...?
No, not yet |
|
Landing this with the caveat that it might need adjustment to support IOS cross-compilation via Xcode. At the moment it seems to have not broken existing workflows and has (locally) enabled universal builds on macOS. |
As requested at the last Halide dev meeting, I have drafted an implementation of support for CMAKE_OSX_ARCHITECTURES. This is again quite complicated, in part because I attempted to avoid breaking existing user code.
As it stands, multi-target alternative selection and feature specification are stepping on each others' toes.
I'll write more tomorrow.
Fixes #8134
Breaking suggestion:
TARGETSargument and theHalide_TARGETvariable shall take only the following values:host(passed verbatim to the generator, but resolved for feature selection)cmakeARCH-BITS-OS(a triple)CMAKE_OSX_ARCHITECTURESis set,cmakeexpands to a list of compatible architectures, e.g.arm-64-osx;x86-64-osx. This is reflected inHalide_CMAKE_TARGET.hostexpands to a single triple corresponding to the architecture of thecmakeexecutable itself (this enables compatibility with thearchcommand).FEATURESargument applies a list of features to ALL alternatives on ALL platforms (useful for, e.g.user_context, name mangling, etc.)FEATURES[platform]arguments set up a list of alternatives (e.g.FEATURES[x86-64-linux] "avx2-sse4" "sse4") for a particular triple.What this doesn't resolve is how to do things like
TARGETS cmake-no_bounds_query cmakeeffectively. Perhaps the following adjustments to the above help:FEATURES[platform]toMULTITARGET[platform]MULTITARGET(unadorned) to provide the default value for unspecified[platform]s.Specifying an empty entry in
FEATURES/MULTITARGETor whatever is challenging in CMake because it really wants to drop empty function arguments. We might need to add a dummy feature likenone(either in the actual Generator or just in the build system) that does nothing but isn't the empty string.