Skip to content
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

Merged
merged 1 commit into from
Jul 13, 2020
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
9 changes: 9 additions & 0 deletions Make.inc
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,11 @@ ifeq ($(ARCH),amd64)
override ARCH := x86_64
endif

# We map arm64 (Apple spelling) to aarch64 to avoid having to deal with both spellings everywhere
ifeq ($(ARCH),arm64)
override ARCH := aarch64
endif

ifeq ($(ARCH),i386)
BINARY:=32
ISX86:=1
Expand Down Expand Up @@ -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
Copy link
Contributor

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...

endif
endif

# Set MARCH-specific flags
Expand Down
10 changes: 8 additions & 2 deletions src/crc32c.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
#include "julia_internal.h"
#include "processor.h"

#ifdef _CPU_AARCH64_
#if defined(_CPU_AARCH64_) && defined(_OS_LINUX_)
# include <sys/auxv.h>
#endif

Expand Down Expand Up @@ -331,7 +331,11 @@ JL_DLLEXPORT uint32_t jl_crc32c(uint32_t crc, const char *buf, size_t len)
{
return crc32c_armv8(crc, buf, len);
}
# else
# elif defined(_OS_DARWIN_)
// All Apple chips that run Darwin should have crc32 support.
// If that ever changes for some reason, this could be detected via the hw.optional.crc32 sysctl
# error Darwin/ARM64, but no CRC32 support?
# elif defined(_OS_LINUX_)
static crc32c_func_t crc32c_dispatch(unsigned long hwcap)
{
if (hwcap & (1 << JL_AArch64_crc))
Expand All @@ -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.
Copy link
Contributor

@yuyichao yuyichao Jul 11, 2020

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..

Copy link
Member Author

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.

Copy link
Contributor

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...)

# endif
#else
// If we don't have any accelerated version to define, just make the _sw version define
Expand Down