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

Enable macOS + Arm64/Neon target for M1 Macs #1943

Closed
wants to merge 0 commits into from

Conversation

Twinklebear
Copy link
Contributor

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

Copy link
Collaborator

@dbabokin dbabokin left a 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.

src/ispc.cpp Outdated Show resolved Hide resolved
src/ispc.cpp Outdated Show resolved Hide resolved
@Twinklebear
Copy link
Contributor Author

Twinklebear commented Dec 11, 2020

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.

@Twinklebear
Copy link
Contributor Author

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

src/ispc.cpp Outdated Show resolved Hide resolved
@dbabokin
Copy link
Collaborator

I guess it's changing the name of arch, which screwed the things up. So even though arm64 is a synonym for aarch64, looks like it's not that simple. I've committed the change that reverts it. Try it now. I don't remember any other changes that might affect it.

Also, rebasing to ispc:master should solve Appveyor problem, as I've turned off macOS cross target on Windows and Linux.

@Twinklebear
Copy link
Contributor Author

Twinklebear commented Dec 20, 2020

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

@dbabokin
Copy link
Collaborator

How do you compile examples?
This command line seems to produce proper ARM assembly (on my intel-based Mac, with LLVM 11).

bin/ispc foo.ispc --target=neon --emit-asm -o foo.s

@Twinklebear
Copy link
Contributor Author

Pretty much the same way, just with --target=neon-i32x4 --arch=aarch64 and outputting an object file instead of asm. Let me check what asm comes out with LLVM 11, will need a second to rebuild it haha

@dbabokin
Copy link
Collaborator

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.

@dbabokin
Copy link
Collaborator

I've built with new (BigSur) SDK and can reproduce it now:

> cmake .. -DISPC_MACOS_SDK_PATH=/Library/Developer/CommandLineTools/SDKs/MacOSX11.0.sdk
> make
<...>
Error: Unknown arch.
make[2]: *** [examples/aobench_instrumented/ao_instrumented_ispc.h] Error 1

@Twinklebear
Copy link
Contributor Author

Twinklebear commented Dec 21, 2020

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:

[ 42%] Built target ispc
[ 42%] Linking CXX executable ../../bin/aobench
ld: warning: ignoring file ao_ispc.o, building for macOS-arm64 but attempting to link with file built for unknown-x86_64
Undefined symbols for architecture arm64:
  "_ao_ispc", referenced from:
      _main in ao.cpp.o
  "_ao_ispc_tasks", referenced from:
      _main in ao.cpp.o
ld: symbol(s) not found for architecture arm64
clang-11: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [bin/aobench] Error 1
make[1]: *** [examples/aobench/CMakeFiles/aobench.dir/all] Error 2
make: *** [all] Error 2

If I just use the ispc compiler built with LLVM 11 to compile an object file:

./bin/ispc ../examples/aobench/ao.ispc --target=neon-i32x4 --arch=aarch64 -o ao.o

Then the output object file is actually for x86_64:

% file ao.o
ao.o: Mach-o 64-bit object x86_64

src/ispc.cpp Outdated Show resolved Hide resolved
@dbabokin
Copy link
Collaborator

I'm not sure why it was working before :) Should work now, at least on my Mac it works.

@dbabokin
Copy link
Collaborator

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

@dbabokin
Copy link
Collaborator

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.

@Twinklebear
Copy link
Contributor Author

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

@Twinklebear
Copy link
Contributor Author

No worries about the force push/closing of the PR

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.

None yet

2 participants