Skip to content

Conversation

@jwatte
Copy link

@jwatte jwatte commented Dec 22, 2021

It turns out, MACOS is a perfectly fine OS to support aarch64 (through its name
arm64.)

The only thing missing was the appropriate check in impl_aarch64, plus renaming
the file to match that it's really for any OS that has the CPU.

jwatte@Jons-MBP build % make test
Running tests...
Test project /Users/jwatte/cpu_features/build
Start 1: bit_utils_test
1/4 Test #1: bit_utils_test ................... Passed 0.06 sec
Start 2: string_view_test
2/4 Test #2: string_view_test ................. Passed 0.04 sec
Start 3: stack_line_reader_test
3/4 Test #3: stack_line_reader_test ........... Passed 0.04 sec
Start 4: cpuinfo_aarch64_test
4/4 Test #4: cpuinfo_aarch64_test ............. Passed 0.12 sec

It turns out, MACOS is a perfectly fine OS to support aarch64 (through its name
arm64.)

The only thing missing was the appropriate check in impl_aarch64, plus renaming
the file to match that it's really for any OS that has the CPU.

jwatte@Jons-MBP build % make test
Running tests...
Test project /Users/jwatte/cpu_features/build
    Start 1: bit_utils_test
1/4 Test google#1: bit_utils_test ...................   Passed    0.06 sec
    Start 2: string_view_test
2/4 Test google#2: string_view_test .................   Passed    0.04 sec
    Start 3: stack_line_reader_test
3/4 Test google#3: stack_line_reader_test ...........   Passed    0.04 sec
    Start 4: cpuinfo_aarch64_test
4/4 Test google#4: cpuinfo_aarch64_test .............   Passed    0.12 sec
@jwatte
Copy link
Author

jwatte commented Dec 22, 2021

Also, I have signed the CLA, and trying to re-sign it gives me an error saying it's already signed.


#ifdef CPU_FEATURES_ARCH_AARCH64
#if defined(CPU_FEATURES_OS_LINUX) || defined(CPU_FEATURES_OS_ANDROID)
#if defined(CPU_FEATURES_OS_LINUX) || defined(CPU_FEATURES_OS_ANDROID) || defined(CPU_FEATURES_OS_MACOS)
Copy link
Contributor

@toor1245 toor1245 Dec 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jwatte, recently added new code layout #194 to separate platform code, current file src/impl_linux_or_android_aarch64.c works with /proc/cpuinfo and /proc/self/auxv which is only suitable for Linux, so this defined(CPU_FEATURES_OS_MACOS) should be in other file src/impl_aarch64_macos.c, also a PR #204 has already been created for this, the only thing is that the PR is big and we will merge it with small PRs

сс: #210

@toor1245
Copy link
Contributor

toor1245 commented Feb 1, 2022

@gchatelet, can we close this PR, since we will be adding changes to fix the build for M1 in the future?

@testn
Copy link

testn commented Mar 7, 2022

Can we get this fixed, please? It has been broken for a long time.

@technicalpickles
Copy link

I think this can be closed now that #209 merged which makes a similar fix

@Mizux
Copy link
Collaborator

Mizux commented Aug 18, 2022

closing it as duplicate of the on going PR:

@Mizux Mizux closed this Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants