-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Darwin/ARM64: Set default mcpu target #36624
Conversation
And unconditionally require the CRC32 extension on this platform. All current and future chips should have it.
@@ -853,6 +858,10 @@ endif | |||
ifneq (,$(findstring aarch64,$(ARCH))) | |||
OPENBLAS_DYNAMIC_ARCH:=0 | |||
OPENBLAS_TARGET_ARCH:=ARMV8 | |||
ifeq ($(OS),Darwin) | |||
# Apple Chips are all at least A12Z | |||
MCPU:=apple-a12 |
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.
So the main issue I worry about this here is that this could override the setting of the compiler if it is ever changed to a different one.
It's certainly much less of an issue on apple platform where there are only going to be relatively few platforms to keep track of and it can still be overwritten if necessary by setting MCPU
on the make command line so I guess this is OK...
@@ -341,6 +345,8 @@ static crc32c_func_t crc32c_dispatch(unsigned long hwcap) | |||
// For ifdef detection below | |||
# define crc32c_dispatch() crc32c_dispatch(getauxval(AT_HWCAP)) | |||
# define crc32c_dispatch_ifunc "crc32c_dispatch" | |||
# else | |||
# warning CRC32 feature detection not implemented for this OS. Falling back to software version. |
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.
Hmm, time to rebase my local crc32 branch for 32bit arm I guess... ;-p
edit: nvm, this is aarch64 only..
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.
Isn't thins whole thing guarded by CPU_AARCH64? Not sure I understand the 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.
Yeah, I though thihs ifdef was one level up (see edit...)
And unconditionally require the CRC32 extension on this platform. All current and future chips should have it.
And unconditionally require the CRC32 extension on this platform.
All current and future chips should have it.