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
Enable macOS + Arm64/Neon target for M1 Macs #1943
Conversation
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.
Thanks for the contribution. The changes generally look good with except the minor change mention below.
We definitely would like to support M1 chip, this will probably require also adding new CPU definition, though I'm afraid it will be supported only by LLVM trunk.
CI fails are due to MacOSX SDK used for cross build, we should update it to the latest to make it work.
I also would like to postpone merging this commit before I can test it myself (I already ordered hardware), because we have a minor release coming next week and I don't want to release with new target that I cannot test myself.
Thanks @dbabokin ! I fixed those issues, no problem on postponing the merge until you can test it more. In this PR I really just tried making it not an error to have macOS + arm and things seemed to build and run ok. I am able to run the ISPC examples and Embree + ISPC stuff with this but I didn't do any in depth validation testing. It'll be great to have official M1 support in ISPC. |
Just a small update, I tried merging this with the latest master to bring in the changes for 1.15 and noticed that ISPC was outputting x86_64 binaries instead of aarch64, so there may be something conflicting there or with llvm 11 that needs to change to output Arm binaries correctly again |
I guess it's changing the name of arch, which screwed the things up. So even though Also, rebasing to ispc:master should solve Appveyor problem, as I've turned off macOS cross target on Windows and Linux. |
It looks like it might be related to a change from LLVM 10 to 11, when I build this PR rebased on top of master, but change the build script to still fetch LLVM 10 I'm able to build the examples. When using LLVM 11, I get the same link error that the object files were compiled for x86_64. Update: I rebuilt master + this PR using LLVM 10 and the object files were compiled correctly for aarch64. So it seems like something tweaked in the change from 10 to 11 |
How do you compile examples?
|
Pretty much the same way, just with |
I was wrong, I tried it on Linux machine, instead of Mac (I need more coffee this morning). My Mac is 10.15, while for M1 support I'll need to update to BigSur. |
I've built with new (BigSur) SDK and can reproduce it now:
|
I think that's the same I'm getting then, now that I've got it rebuilt with LLVM 11 the full error when trying to build the examples is:
If I just use the ispc compiler built with LLVM 11 to compile an object file:
Then the output object file is actually for x86_64:
|
I'm not sure why it was working before :) Should work now, at least on my Mac it works. |
@Twinklebear The changes look good now, but let me add tests and do some clean up. I'll push-force rebased / cleaned up changes to your branch. |
Hmmmm, doing push-force to the source branch of this PR I occasionally push "master", instead of "my branch" to target "master". So PR closed and I can't updated the branch anymore. I've resubmitted the changes in PR #1954. |
Shoot I'm not sure how it was working earlier before either based on that line haha. I'm just rebuilding to check now but https://github.com/ispc/ispc/pull/1954/files looks good |
No worries about the force push/closing of the PR |
This PR enables support for a macOS + Arm64 target to support M1 based macs.
I think this PR is needed to enable #1930 but I'm not sure it resolves that, I didn't test creating a fat binary for x86 + arm. That might need some additional work, but this PR will at least allow the compilation of a macOS + arm binary with ISPC.
This turned out to be a really minor tweak, but let me know if anything else should be changed here