Add feature detection for ARM/MacOS #41924

gbaraldi · 2021-08-18T22:31:28Z

Attempt at fixing #40876 . The fix is a bit of bodge but it's what other languages are doing. Feature detection on macos + arm is quite hard apparently, there are some options but they are very incomplete. The cpu is targeting the a-14 since the m1 target isn't in our LLVM yet.

vtjnash · 2021-08-24T22:37:00Z

LGTM. @Keno as our developer-with-an-M1, can you confirm this PR looks good?

ViralBShah · 2021-11-09T21:30:36Z

@gbaraldi Could you rebase this to master? @Keno can you merge it if it looks good?

gbaraldi · 2021-11-09T21:41:41Z

It looks like the new M1 processors have the same cores but more of them so it should be correct for them too. When LLVM13 merges they can also get the correct name since that's where the m1 name gets added.

edit: Also who do we have to nag so they add a new scheduler for the newer chips. LLVM uses the one from the a7-cyclone which is almost 10 years old

gbaraldi · 2021-12-03T00:40:31Z

Bump 😄

ViralBShah · 2021-12-03T02:29:45Z

Perhaps @vchuravy or @yuyichao may also be able to review.

src/processor_arm.cpp

yuyichao · 2021-12-04T08:45:51Z

src/processor_arm.cpp

+    CPUID info = {
+            uint8_t(0x61),
+            uint8_t(0),
+            uint16_t(0x21)


I don't think you should try to reuse this code and should instead replace the whole function (_get_host_cpu).

But if you do, you should use the M1 info, i.e. 0x23.

src/processor_arm.cpp

yuyichao

And to add,

It could be possible to do it programmatically using developer.apple.com/documentation/kernel/1387446-sysctlbyname but that would necessitate a refactor of the code since I think it just expects linux code for now.

Not really. While the current code is linux-only,

For the feature mask part, there's really no other standard format, (and AFAICT apple doesn't have that) and if there is one at some point it's a simple bit shuffling to translate during detection time.
For the detection, it's intentionally made to be completely independent of the other logic, hence why I said you should replace that whole logic with your own. The linux one cover orders of magnitude more chips than what mac on arm would need and there's no point reusing it. You just need to produce the right name, feature pair.

For future expansion on arm mac, you most likely only need to switch between a few different processors with known feature set. If you really need to actually do feature detection, all you need to do then is to adjust the feature mask accordingly based on sysctlbyname results.

yuyichao · 2021-12-05T11:58:40Z

src/processor_arm.cpp

+#if defined _CPU_AARCH64_ && defined _OS_DARWIN_
+static NOINLINE std::pair<uint32_t,FeatureList<feature_sz>> _get_host_cpu()
+{
+    FeatureList<feature_sz> features = {};


return std::make_pair(CPU::apple_m1, Feature::apple_m1);

There's no need to go through the lookup; I doubt the ELF arch lookup applies to apple (and if anything it won't be elf arch...) and you'll never hit the empty case anyway; no need to lookup features from the CPU since you know what it is; there's also no need to mask any features since the extra features comes from auxv and you aren't getting that it from there.

You should be able to move arm_arch (along with the namespace), get_elf_arch, feature_arch_version, generic_for_arch, check_cpu_arch_ver, shrink_big_little, into the ifdef below as well.

Edit: also you can ifdef out old line 672 to old line 1020 but please keep the change you made in that range since it's for linux.

Wrote some simpler smaller logic that also shouldn't make it too hard to add new apple cores when they launch someday.

yuyichao · 2021-12-05T17:39:54Z

src/processor_arm.cpp

+    if(strcmp(buffer,"Apple M1") == 0)
+        return CPU::apple_m1;
+    else
+        return CPU::generic;// Firestorm core data based on https://opensource.apple.com/source/xnu/xnu-7195.141.2/osfmk/arm/cpuid.h.auto.html


This comment seems to be entirely out of place. It is where I get the information about the CPUID though so if you don't mind you can update the URL on new line 1024.

yuyichao · 2021-12-05T17:42:42Z

src/processor_arm.cpp

+{
+    char buffer[128];
+    size_t bufferlen = 128;
+    sysctlbyname("machdep.cpu.brand_string",&buffer,&bufferlen,NULL,0);


This is a good function to keep for future reference, but the fallback should be CPU::apple_m1. As the code is currently written, the next generation of chip will be detected as generic and I don't think that's desired. The linux version gets around this as much as possible by doing a full feature detection (so the only thing missing would be scheduling model that we can't do that much about...) but there's nothing like that here. I highly doubt apple will release a new processor for mac that has fewer userspace CPU features than M1 so it should be safe to assume so. And it seems to be what other projects assumes as well.

yuyichao · 2021-12-05T17:44:36Z

src/processor_arm.cpp

@@ -20,7 +20,10 @@
 #    include <sys/auxv.h>
 #  endif
 #endif
-
+#if defined _CPU_AARCH64_ && defined _OS_DARWIN_


Not that important but I think you can merge this with the getauxval block above.

yuyichao · 2021-12-05T17:50:02Z

src/processor_arm.cpp

+    size_t bufferlen = 128;
+    sysctlbyname("machdep.cpu.brand_string",&buffer,&bufferlen,NULL,0);
+    if(strcmp(buffer,"Apple M1") == 0)
+        return CPU::apple_m1;


And what I was writing previously would just corresponds to return {CPU::apple_m1, Feature::apple_m1}; in this branch and returning a different pair if a different cpu is detected. Going through find_cpu is also fine, just a bit unnecessary.....

I agree, I think it's cleaner now

yuyichao

LGTM now.

Now I do believe that the developer transition kit does have an older CPU so if we want to support that we could add it explicitly and I assume @Keno would know what the brand string from that is. It should affect almost no real users and for any models in the future, the M1 should still be the sane fallback so this should still be good to go as is.

gbaraldi · 2022-01-20T00:00:03Z

Bump :)

src/processor_arm.cpp

dnadlinger · 2022-01-27T22:27:27Z

+1; I've been running with a patch similar to this (but without any runtime detection) on my M1.

dnadlinger · 2022-01-28T01:38:15Z

src/processor_arm.cpp

+    if(strcmp(buffer,"Apple M1") == 0)
+        return std::make_pair((uint32_t)CPU::apple_m1, Feature::apple_m1);
+    else if(strcmp(buffer,"Apple M1 Max") == 0)
+        return std::make_pair((uint32_t)CPU::apple_m1, Feature::apple_m1);
+    else if(strcmp(buffer,"Apple M1 Pro") == 0)
+        return std::make_pair((uint32_t)CPU::apple_m1, Feature::apple_m1);
+    else
+        return std::make_pair((uint32_t)CPU::apple_m1, Feature::apple_m1);


Hmm, while I appreciate the intent to keep everything explicit, I wonder whether the repetition doesn't just add more clutter than necessary for now. Also, note that Apple themselves recommend parsing hw.optional to detect CPU features, although I suppose the number of desktop(-ish) Apple CPUs will remain managable for a number of years to come.

Parsing it is probably more complicated than just hardcoding things IMO, specially because the effort to add new features to an eventual parser is probably the same as just adding a new CPU to the list.

vchuravy · 2022-02-14T19:20:19Z

@gbaraldi can you rebase this on master once more? And then we should merge this.

gbaraldi and others added 3 commits August 18, 2021 19:26

Add feature detection for ARM/MacOS

223ae7e

Delete md5

5f731c5

Delete sha512

5c4e7f4

ararslan added the system:apple silicon Affects Apple Silicon only (Darwin/ARM64) - e.g. M1 and other M-series chips label Aug 18, 2021

gbaraldi and others added 4 commits August 18, 2021 19:59

Change to Permalink

0b165f0

Change to Permalink

efe722c

Fixing whitespaces

8295ffb

Fix whitespaces

399d22c

gbaraldi marked this pull request as ready for review August 19, 2021 12:08

vtjnash requested a review from Keno August 24, 2021 22:37

Merge branch 'JuliaLang:master' into master

2fddfba

Merge branch 'JuliaLang:master' into master

ed30203

gbaraldi mentioned this pull request Dec 3, 2021

Make DemoteFloat16 a conditional pass #43327

Merged

yuyichao requested changes Dec 4, 2021

View reviewed changes

src/processor_arm.cpp Outdated Show resolved Hide resolved

gbaraldi added 5 commits December 4, 2021 12:24

Merge branch 'JuliaLang:master' into master

9e3ffe8

Add review changes

d96e2ed

Fix LLVM versions

a3f5904

Whistespacing fun

4ecb98d

Fix preprocessor warning

498bac7

yuyichao requested changes Dec 5, 2021

View reviewed changes

gbaraldi added 3 commits December 5, 2021 11:21

Specific logic for apple arm

b44c8e2

Move some ifdefs around

6271618

whitespace

aec01a6

yuyichao requested changes Dec 5, 2021

View reviewed changes

yuyichao reviewed Dec 5, 2021

View reviewed changes

gbaraldi added 2 commits December 5, 2021 17:50

simplifying the code

98c9fa6

Fix URL

b41771d

yuyichao approved these changes Dec 6, 2021

View reviewed changes

dnadlinger mentioned this pull request Jan 5, 2022

Darwin/ARM64: Julia freezes on nested @threads loops #41820

Closed

Merge branch 'JuliaLang:master' into master

ee77b8a

Keno reviewed Jan 20, 2022

View reviewed changes

src/processor_arm.cpp Show resolved Hide resolved

Add M1 Pro and Max

0053128

vchuravy added this to the 1.8 milestone Jan 20, 2022

dnadlinger reviewed Jan 28, 2022

View reviewed changes

vchuravy added the merge me PR is reviewed. Merge when all tests are passing label Feb 14, 2022

Merge branch 'JuliaLang:master' into master

71d21f4

vchuravy merged commit 9542577 into JuliaLang:master Feb 15, 2022

gbaraldi mentioned this pull request Feb 16, 2022

[CPUID] Add ISA entries for A64FX and M1 #44194

Merged

antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request Feb 17, 2022

Add feature detection for ARM/MacOS (JuliaLang#41924)

8566ac4

DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Feb 19, 2022

LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022

Add feature detection for ARM/MacOS (JuliaLang#41924)

bbf6202

LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022

Add feature detection for ARM/MacOS (JuliaLang#41924)

5dfcf9f

giordano mentioned this pull request Apr 6, 2022

Feature/CPU Detection for Apple M1 #40876

Closed

LilithHafner mentioned this pull request Oct 23, 2024

Correction of lcm() for Array arguments #56113

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add feature detection for ARM/MacOS #41924

Add feature detection for ARM/MacOS #41924

gbaraldi commented Aug 18, 2021 •

edited

Loading

vtjnash commented Aug 24, 2021

ViralBShah commented Nov 9, 2021

gbaraldi commented Nov 9, 2021 •

edited

Loading

gbaraldi commented Dec 3, 2021

ViralBShah commented Dec 3, 2021

yuyichao Dec 4, 2021

yuyichao left a comment

yuyichao Dec 5, 2021 •

edited

Loading

gbaraldi Dec 5, 2021

yuyichao Dec 5, 2021

yuyichao Dec 5, 2021 •

edited

Loading

yuyichao Dec 5, 2021

yuyichao Dec 5, 2021

gbaraldi Dec 5, 2021

yuyichao left a comment

gbaraldi commented Jan 20, 2022

dnadlinger commented Jan 27, 2022

dnadlinger Jan 28, 2022

gbaraldi Jan 28, 2022 •

edited

Loading

vchuravy commented Feb 14, 2022

Add feature detection for ARM/MacOS #41924

Add feature detection for ARM/MacOS #41924

Conversation

gbaraldi commented Aug 18, 2021 • edited Loading

vtjnash commented Aug 24, 2021

ViralBShah commented Nov 9, 2021

gbaraldi commented Nov 9, 2021 • edited Loading

gbaraldi commented Dec 3, 2021

ViralBShah commented Dec 3, 2021

yuyichao Dec 4, 2021

Choose a reason for hiding this comment

yuyichao left a comment

Choose a reason for hiding this comment

yuyichao Dec 5, 2021 • edited Loading

Choose a reason for hiding this comment

gbaraldi Dec 5, 2021

Choose a reason for hiding this comment

yuyichao Dec 5, 2021

Choose a reason for hiding this comment

yuyichao Dec 5, 2021 • edited Loading

Choose a reason for hiding this comment

yuyichao Dec 5, 2021

Choose a reason for hiding this comment

yuyichao Dec 5, 2021

Choose a reason for hiding this comment

gbaraldi Dec 5, 2021

Choose a reason for hiding this comment

yuyichao left a comment

Choose a reason for hiding this comment

gbaraldi commented Jan 20, 2022

dnadlinger commented Jan 27, 2022

dnadlinger Jan 28, 2022

Choose a reason for hiding this comment

gbaraldi Jan 28, 2022 • edited Loading

Choose a reason for hiding this comment

vchuravy commented Feb 14, 2022

gbaraldi commented Aug 18, 2021 •

edited

Loading

gbaraldi commented Nov 9, 2021 •

edited

Loading

yuyichao Dec 5, 2021 •

edited

Loading

yuyichao Dec 5, 2021 •

edited

Loading

gbaraldi Jan 28, 2022 •

edited

Loading