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

Add support for Apple M1 AArch64 SoCs #204

Merged
merged 4 commits into from
Aug 28, 2023

Conversation

arkivm
Copy link
Contributor

@arkivm arkivm commented Nov 18, 2021

Completely based on #150. Thanks to @sbehnke!

  • Refactoring to accomodate the new source tree
  • Adding more feature flags

As #150 is stale, I am just trying to push it across the finish line.

Copy link

@sbehnke sbehnke left a comment

Choose a reason for hiding this comment

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

Awesome! Looks good to me. I'll have to give It a try as well. Sorry I was not able to get back to it. Life gets in the way.

@arkivm
Copy link
Contributor Author

arkivm commented Nov 18, 2021

No problem. Thanks for the great work. As numerous projects started tagging that they depend on this, I decided to push it forward.

@arkivm
Copy link
Contributor Author

arkivm commented Nov 18, 2021

As @sbehnke did all the work, can someone tell me how to add something like Linux's Co-developed-by tag to a github PR? I would like @sbehnke to sign off this patch and I can add myself as a co-developer.

Copy link

@sbehnke sbehnke left a comment

Choose a reason for hiding this comment

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

➜  build git:(aarch64-darwin-support) make test 
Running tests...
Test project /Users/sbehnke/cpu_features/build
    Start 1: bit_utils_test
1/4 Test #1: bit_utils_test ...................   Passed    0.13 sec
    Start 2: string_view_test
2/4 Test #2: string_view_test .................   Passed    0.08 sec
    Start 3: stack_line_reader_test
3/4 Test #3: stack_line_reader_test ...........   Passed    0.07 sec
    Start 4: cpuinfo_aarch64_test
4/4 Test #4: cpuinfo_aarch64_test .............   Passed    0.07 sec

100% tests passed, 0 tests failed out of 4

Total Test time (real) =   0.35 sec
➜  build git:(aarch64-darwin-support) ./list_cpu_features 
arch            : aarch64
implementer     : 16777228 (0x100000C)
variant         :   2 (0x02)
part            : 458787763 (0x1B588BB3)
revision        :   5 (0x05)
flags           : aes,asimd,asimdfhm,atomics,crc32,fcma,fp,fphp,jscvt,lrcpc,pmull,sb,sha1,sha3,sha512,ssbs

Looks good on my M1 Max.

@arkivm
Copy link
Contributor Author

arkivm commented Nov 18, 2021

Looks good on my M1 Max.

Great. Thank you for quickly testing it.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
src/impl_aarch64_macos.c Outdated Show resolved Hide resolved
@toor1245 toor1245 mentioned this pull request Nov 18, 2021
@toor1245
Copy link
Contributor

toor1245 commented Nov 18, 2021

I don't see the __arm64__ macro for CPU_FEATURES_ARCH_AARCH64 in cpu_features_macros.h? This macro is used for iphone

src/impl_aarch64_macos.c Outdated Show resolved Hide resolved
@arkivm arkivm force-pushed the aarch64-darwin-support branch 2 times, most recently from 438c1ae to 57badab Compare November 18, 2021 20:34
@arkivm
Copy link
Contributor Author

arkivm commented Nov 18, 2021

I don't see the __arm64__ macro for CPU_FEATURES_ARCH_AARCH64 in cpu_features_macros.h? This macro is used for iphone

added this as well.

src/impl_aarch64_macos.c Outdated Show resolved Hide resolved
@arkivm
Copy link
Contributor Author

arkivm commented Nov 22, 2021

bump

@gchatelet
Copy link
Collaborator

Thx for the work here @arkivm we now have a clearer vision of what's needed to support Apple M1. That said I find the PR too big to be pulled in as-is. I'd rather implement it by creating many small PR that fixes individual issues (see #209 ). Let's keep the PR around for now while we fix it incrementally.

@SchrodingerZhu
Copy link

How is the progress of merging?

@Mizux Mizux self-assigned this Aug 18, 2022
@Mizux Mizux added the Apple M1 Apple M1 related issues label Aug 18, 2022
toor1245 pushed a commit to toor1245/cpu_features that referenced this pull request Aug 18, 2022
To add support AARCH64 for other operating systems such as macOS(Apple M1), ios, FreeBSD, Windows has been moved common logic from 
`src/impl_aarch64_linux_or_android.c` to `src/impl_aarch64__base_implementation.inl`, namely:
* Definitions for introspection
* `Aarch64Info` kEmptyAarch64Info field

Removed include "internal/bit_utils.h" from `src/impl_aarch64_linux_or_android.c`, since this include was not used. Also, include `cpuinfo_aarch64` has been removed  from linux implementation and replaced with `impl_aarch64__base_implementation.inl`, this include will be used for all other operating system impl as well

Added a compilation check that matches the base X86 implementation

Refs: google#121
See also: google#150, google#186, google#204
toor1245 pushed a commit to toor1245/cpu_features that referenced this pull request Aug 18, 2022
To add support AARCH64 for other operating systems such as macOS(Apple M1), ios, FreeBSD, Windows has been moved common logic from 
`src/impl_aarch64_linux_or_android.c` to `src/impl_aarch64__base_implementation.inl`, namely:
* Definitions for introspection
* `Aarch64Info` kEmptyAarch64Info field

Removed include "internal/bit_utils.h" from `src/impl_aarch64_linux_or_android.c`, since this include was not used. Also, include `cpuinfo_aarch64` has been removed from linux implementation and replaced with `impl_aarch64__base_implementation.inl`, this include will be used for all other operating systems impl as well

Added a compilation check that matches the base X86 implementation

Refs: google#121
See also: google#150, google#186, google#204
@toor1245 toor1245 mentioned this pull request Aug 30, 2022
mscdex added a commit to mscdex/cpu-features that referenced this pull request Aug 17, 2023
These changes were originally pulled from:

google/cpu_features#204

adapted to be compatible with cpu_features v0.8.0 and
with the addition of the ECV CPU feature for Apple Silicon.
@mscdex
Copy link

mscdex commented Aug 17, 2023

Would it be possible to land this soon?

For what it's worth I've gone ahead and applied the changes (fixing several merge conflicts) on top of v0.8.0 and I have had at least one M1 user report that it works (I do not have access to Apple Silicon hardware myself). If anyone is interested in these changes, they can be found in the patch here.

mscdex added a commit to mscdex/cpu-features that referenced this pull request Aug 17, 2023
These changes were originally pulled from:

google/cpu_features#204

adapted to be compatible with cpu_features v0.8.0 and
with the addition of the ECV CPU feature for Apple Silicon.
@gchatelet
Copy link
Collaborator

I fixed the patch merging. @Mizux would you be able to test this patch on Apple silicon? Then it should be good to go.

@gchatelet gchatelet added this to the v0.9.0 milestone Aug 24, 2023
@gchatelet gchatelet added the enhancement New feature or request label Aug 24, 2023
@Mizux
Copy link
Collaborator

Mizux commented Aug 24, 2023

FYI, I'm currently rebasing this branch on main since the merge of the PR for aarch64 Windows conflict...
EDIT: my bad rebase on main is borken but merging was ok

I have a Apple M1 and one Intel so I can test ;)

@Mizux
Copy link
Collaborator

Mizux commented Aug 24, 2023

[^v^]─corentinl@corentinl-macbookpro2 %CTEST_OUTPUT_ON_FAILURE=1 cmake --build build --target test -v
Running tests...
/opt/homebrew/Cellar/cmake/3.26.4/bin/ctest --force-new-ctest-process 
Test project /Users/corentinl/work/cpu_features/build
    Start 1: bit_utils_test
1/4 Test #1: bit_utils_test ...................   Passed    0.00 sec
    Start 2: string_view_test
2/4 Test #2: string_view_test .................   Passed    0.00 sec
    Start 3: stack_line_reader_test
3/4 Test #3: stack_line_reader_test ...........   Passed    0.00 sec
    Start 4: cpuinfo_aarch64_test
4/4 Test #4: cpuinfo_aarch64_test .............   Passed    0.65 sec

100% tests passed, 0 tests failed out of 4
[0]─[~/work/cpu_features]-[aarch64-darwin-support]
[^u^]─corentinl@corentinl-macbookpro2 %build/test/cpuinfo_aarch64_test 
Running main() from gmock_main.cc
[==========] Running 2 tests from 2 test suites.
[----------] Global test environment set-up.
[----------] 1 test from CpuinfoAarch64Test
[ RUN      ] CpuinfoAarch64Test.Aarch64FeaturesEnum
[       OK ] CpuinfoAarch64Test.Aarch64FeaturesEnum (0 ms)
[----------] 1 test from CpuinfoAarch64Test (0 ms total)

[----------] 1 test from CpuidAarch64Test
[ RUN      ] CpuidAarch64Test.FromDarwinSysctlFromName
[       OK ] CpuidAarch64Test.FromDarwinSysctlFromName (0 ms)
[----------] 1 test from CpuidAarch64Test (0 ms total)

[----------] Global test environment tear-down
[==========] 2 tests from 2 test suites ran. (0 ms total)
[  PASSED  ] 2 tests.

@gchatelet
Copy link
Collaborator

@arkivm @Mizux

Looking at https://github.com/google/cpu_features/pull/204/files#diff-fc9e9a184d90d6afa52754eeef66a041131b11a4fddc7cb8668f7cf54d6d2585R54-R75 I feel like there are still quite a few features that need to be filled in from sysctlbyname. Do we have a way to get a complete list of available options?

@arkivm
Copy link
Contributor Author

arkivm commented Aug 27, 2023

I rebased the patches and updated a few flags that got changed. There are a bunch of additional features on aarch64 apple m2 silicon. Does those features have to match with the Linux's hwcap?

@gchatelet
Copy link
Collaborator

I rebased the patches and updated a few flags that got changed.

Thx !

There are a bunch of additional features on aarch64 apple m2 silicon. Does those features have to match with the Linux's hwcap?

AFAIU both sysctlbyname and hwcap are Linux maintained variables so I hope they are consistent with each other 🤷.
But for instance I don't quite know why we have hw.optional.arm.FEAT_SHA256 vs HWCAP_SHA2...

@Mizux
Copy link
Collaborator

Mizux commented Aug 28, 2023

sysctl on M1: https://gist.github.com/Mizux/fc3309b1efeb1454cc7b093c5f23992e
(ed system_profiler SPHardwareDataType seems cool to identify device model)

@gchatelet
Copy link
Collaborator

Thx @Mizux, the list of features is the same between M1 and M2 but the following ones are 0 on M1 and 1 on M2.

hw.optional.arm.FEAT_PAuth2
hw.optional.arm.FEAT_FPAC
hw.optional.arm.FEAT_BF16
hw.optional.arm.FEAT_I8MM
hw.optional.arm.FEAT_BTI

@gchatelet
Copy link
Collaborator

Ok I think this is good for a first version. Let's iterate if needed. Thx all!

@gchatelet gchatelet merged commit c5ece5e into google:main Aug 28, 2023
27 checks passed
@gchatelet gchatelet mentioned this pull request Aug 28, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Apple M1 Apple M1 related issues enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants