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

Support Windows/macOS #73

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Support Windows/macOS #73

wants to merge 18 commits into from

Conversation

motabbara
Copy link

For this to work I also had to modify Enzyme CMakeLists.txt

Not sure whether these make sense so I guess this is more of a draft PR

CMakeLists.txt

@ZuseZ4
Copy link
Member

ZuseZ4 commented Feb 7, 2024

Thanks a lot! I looked through your CMakeList.txt changes and created this patch, can you make a PR for Enzyme core out of it? (I didn't had your gh email, so I couldn't commit in your name)
patch.diff.txt
Once that is merged I'll update the Enzyme submodule and merge this pr here.

Also, do you know what happened to your cargo.lock? It is ~7 times the original size, did you maybe run x or x.py from that folder? I am not sure why it would otherwise change so much.

Final question, on which systems could you test this? We hope to add CI soonish, but I didn't got to it yet, so only some Linux versions are really tested.
Something like this should print it: rustc -Z unstable-options --print target-spec-json | grep llvm-target

@ZuseZ4
Copy link
Member

ZuseZ4 commented Feb 7, 2024

Also @motabbara, if you are currently free, we are discussing that in our weekly open Enzyme call at https://mit.zoom.us/j/96000853439

Copy link

@jedbrown jedbrown left a comment

Choose a reason for hiding this comment

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

Are the Cargo.lock changes important/significant?

@ZuseZ4 Do you want to merge this into macro2 instead?

@motabbara
Copy link
Author

Are the Cargo.lock changes important/significant?

@ZuseZ4 Do you want to merge this into macro2 instead?

I'll put it into macro2 and remove the lock file noise.

@motabbara
Copy link
Author

Thanks a lot! I looked through your CMakeList.txt changes and created this patch, can you make a PR for Enzyme core out of it? (I didn't had your gh email, so I couldn't commit in your name) patch.diff.txt Once that is merged I'll update the Enzyme submodule and merge this pr here.

Also, do you know what happened to your cargo.lock? It is ~7 times the original size, did you maybe run x or x.py from that folder? I am not sure why it would otherwise change so much.

Final question, on which systems could you test this? We hope to add CI soonish, but I didn't got to it yet, so only some Linux versions are really tested. Something like this should print it: rustc -Z unstable-options --print target-spec-json | grep llvm-target

"llvm-target": "arm64-apple-macosx11.0.0",

@ZuseZ4
Copy link
Member

ZuseZ4 commented Feb 9, 2024

@motabbara, I went ahead and created a similar PR to Enzyme, since I just tried to rebuild rust-enzyme and ended up with similar errors. I additionally to your patch also had to update the BCLoader CMakeLists, and I didn't needed your Mac specific changes, so I left these for you.

let (lib_prefix, lib_ext) = match env::consts::OS {
"macos" => ("lib", "dylib"),
"windows" => ("", "dll"),
_ => ("", "so")
Copy link
Member

Choose a reason for hiding this comment

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

for linux it would also be lib as prefix, as with the dst_lib.
Also, I assume you will also need to update dst_lib with the prefix?

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.

3 participants