Get GMP building on Apple Silicon #57315

BytesGuy · 2020-07-02T22:07:19Z

Have you followed the guidelines for contributing?
Have you checked that there aren't other open pull requests for the same formula update/change?
Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

This is a bit of a halfway fix to get GMP building and working on Apple Silicon. The architecture is not yet supported officially, so while we wait this will produce a generic C build - according to the documentation this may run slowly, but at least it gets it working for now!

claui · 2020-07-02T22:08:46Z

This might be really helpful, given that lots of popular formulae depend on gmp.

BytesGuy · 2020-07-02T22:15:11Z

This might be really helpful, given that lots of popular formulae depend on gmp.

Yeah, it's probably about as good as it gets for now 😄 I started working on this because I was trying to install GCC which depends on it - working on that next!

fxcoudert · 2020-07-03T11:49:23Z

Alternatively there is a patch for this on the gmp-bugs mailing list: https://gmplib.org/list-archives/gmp-bugs/2020-July/004837.html
It would enable the use of aarch64 assembly, making GMP more efficient

BytesGuy · 2020-07-03T16:32:11Z

Alternatively there is a patch for this on the gmp-bugs mailing list: https://gmplib.org/list-archives/gmp-bugs/2020-July/004837.html
It would enable the use of aarch64 assembly, making GMP more efficient

I have just been trying to get this to work with little success for some reason. Are you doing anything noteworthy that I might be missing here? Fairly new to this so apologies if it's something obvious!

Applying the patch and having no --build defined for configure provides:

==> Patching
==> Applying attachment.bin
patching file configure.ac
patching file mpn/arm64/arm64-defs.m4
patching file mpn/arm64/bdiv_q_1.asm
patching file mpn/arm64/darwin.m4
patching file mpn/arm64/invert_limb.asm
==> ./configure
Last 15 lines from /Users/adam/Library/Logs/Homebrew/gmp/01.configure:
checking if globals are prefixed by underscore... yes
checking how to switch to read-only data section... 	.section	__TEXT,__const
checking for assembler .type directive... 
checking for assembler .size directive... 
checking for assembler local label prefix... L
checking for assembler byte directive... .byte
checking how to define a 32-bit word... .long
checking if .align assembly directive is logarithmic... yes
checking size of void *... 8
checking size of unsigned short... 2
checking size of unsigned... 4
checking size of unsigned long... 8
checking size of mp_limb_t... 8
configure: error: Oops, mp_limb_t is 64 bits, but the assembler code
in this configuration expects 32 bits.
/usr/bin/curl --version
/usr/libexec/java_home --xml --failfast
/usr/local/Homebrew/Library/Homebrew/debrew.rb:13:in `raise'

Adding just --build=aarch64-apple-darwin20 to configure provides:

==> Patching
==> Applying attachment.bin
patching file configure.ac
patching file mpn/arm64/arm64-defs.m4
patching file mpn/arm64/bdiv_q_1.asm
patching file mpn/arm64/darwin.m4
patching file mpn/arm64/invert_limb.asm
==> ./configure --build=aarch64-apple-darwin20
==> make
Last 15 lines from /Users/adam/Library/Logs/Homebrew/gmp/02.make:
       ^
tmp-bdiv_q_1.s:76:12: error: expected ')' in parentheses expression
 LEA_LO( x7, ___gmp_binvert_limb_table)
           ^
tmp-bdiv_q_1.s:76:8: error: invalid operand
 LEA_LO( x7, ___gmp_binvert_limb_table)
       ^
make[2]: *** [bdiv_q_1.lo] Error 1
make[2]: *** Waiting for unfinished jobs....
libtool: compile:  clang -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_dcpi1_bdiv_qr -O2 -pedantic -march=armv8-a -c dcpi1_bdiv_qr.c  -fno-common -DPIC -o .libs/dcpi1_bdiv_qr.o
libtool: compile:  clang -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_sbpi1_bdiv_r -O2 -pedantic -march=armv8-a -c sbpi1_bdiv_r.c -o sbpi1_bdiv_r.o >/dev/null 2>&1
libtool: compile:  clang -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_dcpi1_bdiv_q -O2 -pedantic -march=armv8-a -c dcpi1_bdiv_q.c -o dcpi1_bdiv_q.o >/dev/null 2>&1
libtool: compile:  clang -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I.. -DOPERATION_dcpi1_bdiv_qr -O2 -pedantic -march=armv8-a -c dcpi1_bdiv_qr.c -o dcpi1_bdiv_qr.o >/dev/null 2>&1
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2
/usr/bin/curl --version
/usr/libexec/java_home --xml --failfast
/usr/local/Homebrew/Library/Homebrew/debrew.rb:13:in `raise'

fxcoudert · 2020-07-03T17:03:27Z

Since it's a configure.ac patch, it needs to regenerate configure before running it, with:

system "autoreconf", "-fiv"

and build-dependencies on autoconf, automake, and libtool.

claui · 2020-07-03T17:16:38Z

Example:

homebrew-core/Formula/libopendkim.rb

Lines 30 to 31 in 628584b

    
           # Recreate configure due to patch 
        
           system "autoreconf", "-fvi"

claui · 2020-07-03T17:29:00Z

With autoreconf in place, the formula builds now but the static linking test fails.

homebrew-core/Formula/gmp.rb

Lines 50 to 52 in dc859fc

    
           # Test the static library to catch potential linking issues 
        
           system ENV.cc, "test.c", "#{lib}/libgmp.a", "-o", "test" 
        
           system "./test"

Undefined symbols for architecture arm64:
  "__gmpn_pi1_bdiv_q_1", referenced from:
      ___gmpn_bdiv_q_1 in libgmp.a(bdiv_q_1.o)
     (maybe you meant: ___gmpn_pi1_bdiv_q_1)
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

It’s hard to see with some fonts but somehow it seems to be about the number of leading _s, again.

BytesGuy · 2020-07-03T17:53:20Z

@fxcoudert @claui Thank you for the pointers! 🙂 I really need to brush up on these things again, it's been a long time! Just pushed a new commit which now seems to build ok. Tests fail though, as mentioned. Hopefully this is at least a step in the right direction 🤞

claui · 2020-07-03T18:40:34Z

I think the branch statement in bdiv_q_1.asm is what’s causing the error:

LEA_HI(	x7, binvert_limb_table)
ubfx	x6, d, 1, 7
LEA_LO(	x7, binvert_limb_table)
ldrb	w6, [x7, x6]
ubfiz	x7, x6, 1, 8
umull	x6, w6, w6
msub	x6, x6, d, x7
lsl	x7, x6, 1
mul	x6, x6, x6
msub	x6, x6, d, x7
lsl	x7, x6, 1
mul	x6, x6, x6
msub	di, x6, d, x7

b	mpn_pi1_bdiv_q_1    ←

Evidence

$ nm /usr/local/lib/libgmp.a | grep gmpn_pi U ___gmpn_pi1_bdiv_q_1 U ___gmpn_pi1_bdiv_q_1 U ___gmpn_pi1_bdiv_q_1 U ___gmpn_pi1_bdiv_q_1 U ___gmpn_pi1_bdiv_q_1 U ___gmpn_pi1_bdiv_q_1 0000000000000048 T ___gmpn_pi1_bdiv_q_1 U __gmpn_pi1_bdiv_q_1 ← U ___gmpn_pi1_bdiv_q_1 U ___gmpn_pi1_bdiv_q_1

$ nm /usr/local/lib/libgmp.a | grep -A 6 bdiv_q_1.o
/usr/local/lib/libgmp.a(bdiv_q_1.o):
U ___gmp_binvert_limb_table
0000000000000000 T ___gmpn_bdiv_q_1
0000000000000048 T ___gmpn_pi1_bdiv_q_1
U __gmpn_pi1_bdiv_q_1 ←
0000000000000000 t ltmp0

A branch statement in the arm64 flavour of `bdiv_q_1.asm` caused the following error on `brew test`: ``` Undefined symbols for architecture arm64: "__gmpn_pi1_bdiv_q_1", referenced from: ___gmpn_bdiv_q_1 in libgmp.a(bdiv_q_1.o) (maybe you meant: ___gmpn_pi1_bdiv_q_1) ld: symbol(s) not found for architecture arm64 clang: error: linker command failed with exit code 1 (use -v to see invocation) ``` This commit fixes the issue by prepending `GSYM_PREFIX`. It’s warranted to incorporate this patch early because a large number of formulae depends on `gmp`, and waiting for an upstream fix would introduce undue delay in testing those dependent formulae.

BytesGuy

Awesome, thank you! 🎉

claui · 2020-07-04T08:02:46Z

The CI failures in adios2, agda and ncview are obviously unrelated.

claui · 2020-07-04T08:09:30Z

Pinging @fxcoudert for a second review/opinion.

Formula/gmp.rb

fxcoudert

See comments

claui · 2020-07-04T12:41:52Z

Double-checked that the consolidated patch works on arm64.

Waiting on CI (style/audit only) to pass, and for Homebrew/formula-patches#288 to be merged.

This commit consolidates the two patches proposed in Homebrew/homebrew-core#57315. Suggested-by: FX Coudert <fxcoudert@gmail.com>

Formula/gmp.rb

The commit hash for the patch needed an update due to Homebrew/formula-patches#288 being rebased and merged.

claui · 2020-07-06T13:23:14Z

Thanks to all of you! This is a textbook example for excellent collaboration among contributors and with upstream. ❤️

BytesGuy · 2020-07-06T13:30:54Z

Thanks to all of you! This is a textbook example for excellent collaboration among contributors and with upstream. ❤️

Thank you for working on this too! So glad to see this fixed as it should unblock a number of other packages 🎉

claui mentioned this pull request Jul 3, 2020

macOS 11 Big Sur compatibility on Apple Silicon Homebrew/brew#7857

Closed

9 tasks

BytesGuy added 5 commits July 3, 2020 19:10

Get GMP building on Apple Silicon

379f7b6

Update gmp.rb

9665d01

Attempt ASM patch

6496e11

Fix with autoreconf

036c6a2

Resolve style error

1c49840

claui added do not merge test failure CI fails while running the test-do block labels Jul 3, 2020

claui added 2 commits July 3, 2020 22:22

gmp: clarify patch is temporary

a9f4108

BytesGuy commented Jul 3, 2020

View reviewed changes

claui added 11 Big Sur is specifically affected and removed do not merge test failure CI fails while running the test-do block labels Jul 3, 2020

claui previously approved these changes Jul 4, 2020

View reviewed changes

claui requested a review from fxcoudert July 4, 2020 08:08

fxcoudert reviewed Jul 4, 2020

View reviewed changes

Formula/gmp.rb Outdated Show resolved Hide resolved

fxcoudert reviewed Jul 4, 2020

View reviewed changes

Formula/gmp.rb Outdated Show resolved Hide resolved

fxcoudert reviewed Jul 4, 2020

View reviewed changes

Formula/gmp.rb Outdated Show resolved Hide resolved

fxcoudert requested changes Jul 4, 2020

View reviewed changes

Restore config flags and tidy arm depends

7b1d75b

BytesGuy dismissed claui’s stale review via 7b1d75b July 4, 2020 11:27

claui mentioned this pull request Jul 4, 2020

gmp: add arm64 compatibility patch Homebrew/formula-patches#288

Merged

gmp: use consolidated formula patch

77beee8

This comment has been minimized.

Sign in to view

BytesGuy mentioned this pull request Jul 4, 2020

Fix for nettle on Apple Silicon #57455

Merged

5 tasks

miccal mentioned this pull request Jul 6, 2020

libevent 2.1.12 #57506

Closed

claui added a commit to Homebrew/formula-patches that referenced this pull request Jul 6, 2020

gmp: add arm64 compatibility patch

c53834b

This commit consolidates the two patches proposed in Homebrew/homebrew-core#57315. Suggested-by: FX Coudert <fxcoudert@gmail.com>

claui reviewed Jul 6, 2020

View reviewed changes

Formula/gmp.rb Outdated Show resolved Hide resolved

gmp: update patch SHA265 after rebase

aad0270

The commit hash for the patch needed an update due to Homebrew/formula-patches#288 being rebased and merged.

claui approved these changes Jul 6, 2020

View reviewed changes

claui merged commit 2d61d06 into Homebrew:master Jul 6, 2020

BytesGuy deleted the gmp-arm branch July 6, 2020 13:31

claui mentioned this pull request Jul 13, 2020

ghc: apply arm64 compatibility patch to static gmp #57892

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get GMP building on Apple Silicon #57315

Get GMP building on Apple Silicon #57315

BytesGuy commented Jul 2, 2020

claui commented Jul 2, 2020

BytesGuy commented Jul 2, 2020 •

edited

Loading

fxcoudert commented Jul 3, 2020 •

edited

Loading

BytesGuy commented Jul 3, 2020

fxcoudert commented Jul 3, 2020 •

edited

Loading

claui commented Jul 3, 2020 •

edited

Loading

claui commented Jul 3, 2020

BytesGuy commented Jul 3, 2020

claui commented Jul 3, 2020

BytesGuy left a comment

claui commented Jul 4, 2020

claui commented Jul 4, 2020

fxcoudert left a comment

claui commented Jul 4, 2020

This comment has been minimized.

claui commented Jul 6, 2020

BytesGuy commented Jul 6, 2020

Get GMP building on Apple Silicon #57315

Get GMP building on Apple Silicon #57315

Conversation

BytesGuy commented Jul 2, 2020

claui commented Jul 2, 2020

BytesGuy commented Jul 2, 2020 • edited Loading

fxcoudert commented Jul 3, 2020 • edited Loading

BytesGuy commented Jul 3, 2020

fxcoudert commented Jul 3, 2020 • edited Loading

claui commented Jul 3, 2020 • edited Loading

claui commented Jul 3, 2020

BytesGuy commented Jul 3, 2020

claui commented Jul 3, 2020

BytesGuy left a comment

Choose a reason for hiding this comment

claui commented Jul 4, 2020

claui commented Jul 4, 2020

fxcoudert left a comment

Choose a reason for hiding this comment

claui commented Jul 4, 2020

This comment has been minimized.

claui commented Jul 6, 2020

BytesGuy commented Jul 6, 2020

BytesGuy commented Jul 2, 2020 •

edited

Loading

fxcoudert commented Jul 3, 2020 •

edited

Loading

fxcoudert commented Jul 3, 2020 •

edited

Loading

claui commented Jul 3, 2020 •

edited

Loading