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.

Sign up for GitHub

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

Jump to bottom

[GMP] Patch for Apple Silicon #42293

Merged
merged 5 commits into from
Sep 23, 2021
Merged

[GMP] Patch for Apple Silicon #42293

merged 5 commits into from
Sep 23, 2021

Conversation

jeremiahpslewis
Copy link
Contributor

@jeremiahpslewis jeremiahpslewis commented Sep 17, 2021 •
edited
Loading

GMP appears to need a patch for Arm64, thank you to @domschl for finding this!

Ideally, this will resolve #42184

@jeremiahpslewis jeremiahpslewis marked this pull request as draft September 17, 2021 16:03
@vchuravy
Copy link
Member

Can you also add the patch to https://github.com/JuliaPackaging/Yggdrasil/tree/master/G/GMP?

@vchuravy vchuravy added system:apple silicon Affects Apple Silicon only (Darwin/ARM64) - e.g. M1 and other M-series chips backport 1.7 external dependencies Involves LLVM, OpenBLAS, or other linked libraries labels Sep 17, 2021
@jeremiahpslewis
Copy link
Contributor Author

@vchuravy Let me see if I can walk this PR to the finish line, and then I’d gladly do the same for Yggrasil…so far this is more of a shot in the dark based on @domschl’s tip. :)

@mryodo mryodo mentioned this pull request Sep 17, 2021
Closed
@jeremiahpslewis
Copy link
Contributor Author

@mryodo Can you test this on your machine and confirm the build now works?

@mryodo
Copy link

mryodo commented Sep 17, 2021

@mryodo Can you test this on your machine and confirm the build now works?

Sure! Thanks for the work, man, builds perfectly for me on the 1.8.0-DEV
OpenBLAS is still broken though same as on 1.7.0-**, i've reopened the issue #42176

@jeremiahpslewis jeremiahpslewis marked this pull request as ready for review September 17, 2021 17:53
@jeremiahpslewis
Copy link
Contributor Author

Does anyone know if there are license compatibility issues with incorporating this patch into the Julia repo? Afaict gmp is GPL/LGPL...

@jeremiahpslewis jeremiahpslewis mentioned this pull request Sep 17, 2021
Merged
staticfloat
staticfloat reviewed Sep 17, 2021
View reviewed changes
deps/gmp.mk Outdated Show resolved Hide resolved
@stevengj
Copy link
Member

stevengj commented Sep 17, 2021 •
edited
Loading

Can't you just s/x18/x17/g on the file rather than using patch? That will avoid copyright questions. I guess there is no single register rename that works in all the cases?

@stevengj
Copy link
Member

stevengj commented Sep 17, 2021 •
edited
Loading

Has someone reported this upstream to [email protected] yet?

@domschl
Copy link

domschl commented Sep 17, 2021

@stevengj : The patch was provided by upstream.

staticfloat
staticfloat approved these changes Sep 19, 2021
View reviewed changes
Copy link
Member

@staticfloat staticfloat left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@vchuravy
Copy link
Member

Great! Now the adding the patch to Yggrdasil and also bumping the GMB build we use when not building from source would be the next step.

@jeremiahpslewis
Copy link
Contributor Author

Yggdrasil patch is already available, here: JuliaPackaging/Yggdrasil#3640

@jeremiahpslewis
Copy link
Contributor Author

Not sure what you mean about the non-source gmb lib version, can you flesh out your comment?

@vchuravy
Copy link
Member

Yggdrasil patch is already available, here: JuliaPackaging/Yggdrasil#3640

Okay landed.

Can you bump

julia/stdlib/GMP_jll/Project.toml

Line 3 in 52ec8c6

version = "6.2.1+0"
to 6.2.1+1

and then make -f contrib/refresh_checksums.mk GMP (on my phone but hopefully I remembered the right command).

This will update what binary we will pull down when we don't build against the source version. A build from BinaryBuilder is the default (on CI as well)

@jeremiahpslewis
Copy link
Contributor Author

Pushed updates.

@vchuravy
Copy link
Member

Something went wrong. It should have updated https://github.com/JuliaLang/julia/blob/master/deps/checksums/gmp instead of creating many files.

@staticfloat any ideas?

@jeremiahpslewis
Copy link
Contributor Author

@vchuravy didn't let the script run all the way through. Fixed and pushed.

@vchuravy
Copy link
Member

@vchuravy didn't let the script run all the way through. Fixed and pushed.

Not seeing the push, yet :)

@jeremiahpslewis
Copy link
Contributor Author

@vchuravy didn't let the script run all the way through. Fixed and pushed.

Not seeing the push, yet :)

Force push failed on me. Now. :)

@KristofferC KristofferC mentioned this pull request Sep 23, 2021
Merged
84 tasks
@KristofferC KristofferC merged commit 764159c into JuliaLang:master Sep 23, 2021
KristofferC pushed a commit that referenced this pull request Sep 23, 2021
@jeremiahpslewis @KristofferC
[GMP] Patch for Apple Silicon (#42293)
cd462dc 
* [GMP] Patch for Apple Silicon

(cherry picked from commit 764159c)
KristofferC pushed a commit that referenced this pull request Sep 28, 2021
@jeremiahpslewis @KristofferC
[GMP] Patch for Apple Silicon (#42293)
3735240 
* [GMP] Patch for Apple Silicon
@Sacha0 Sacha0 mentioned this pull request Oct 7, 2021
Merged
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
@jeremiahpslewis @LilithHafner
[GMP] Patch for Apple Silicon (JuliaLang#42293)
b495c22 
* [GMP] Patch for Apple Silicon
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
@jeremiahpslewis @LilithHafner
[GMP] Patch for Apple Silicon (JuliaLang#42293)
506f84d 
* [GMP] Patch for Apple Silicon
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@staticfloat staticfloat staticfloat approved these changes

Assignees
No one assigned
Labels
external dependencies Involves LLVM, OpenBLAS, or other linked libraries system:apple silicon Affects Apple Silicon only (Darwin/ARM64) - e.g. M1 and other M-series chips
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Julia fails to build on M1, MacOS 12.0 Beta6
7 participants
@jeremiahpslewis @vchuravy @mryodo @stevengj @domschl @staticfloat @KristofferC