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

ghc: add support for Apple Silicon #65997

Closed
wants to merge 1 commit into from

Conversation

hjelmn
Copy link
Contributor

@hjelmn hjelmn commented Dec 1, 2020

This PR adds support to ghc.rb for Apple Silicon Macs. This PR removes the
external build of gmp in favor of using the in-tree one as the in-tree one
is statically linked these days (confirmed no external gmp references). The
version in-tree is 6.1.2 which does not build on Apple Silicon so it is
replaced by the formula with 6.2.1 which does.

In order to get a full install with a working arm64 binary we currently need
to build ghc in three steps:

  1. Install the ghc binaries for x86_64 into a temporary directory (this is
    also done for x86_64).

  2. Build a cross-compiler for arm64 using the x86_64 binary to bootstrap.

  3. Use either the x86_64 binaries (on x86_64) or the cross-compiler to build
    a native ghc for the system.

This PR relies on an extensive patch based on changes to ghc master. These
patches add support for arm64-apple-darwin and correct some build system
confusion between the ghc target name (aarch64-apple-darwin) and the llvm
target name (arm64-apple-darwin). These patches will be PR'd to the formula
patch repo.

Some caveats:

  • This build takes awhile (over 2 hours). This is a significant build time
    for an M1 system given it is roughly twice as fast as a Core i9.

  • We can probably shorten the build time by using integer-simple and
    disabling other libraries in the cross-compile but this formula is a
    stop-gap measure until upstream releases official support.

  • There is no native code generator for arm64 at this time. The resulting
    binaries will likely be slower than if there was a native code generator.

Signed-off-by: Nathan Hjelm [email protected]

  • 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>)?

@BrewTestBot BrewTestBot added the python Python use is a significant feature of the PR or issue label Dec 1, 2020
@hjelmn
Copy link
Contributor Author

hjelmn commented Dec 1, 2020

BrewTestBot put the wrong label on this one. This is Haskell not Python.

@hjelmn hjelmn force-pushed the ghc_apple_silicon branch from db28c22 to 7921a7d Compare December 1, 2020 15:23
@fxcoudert
Copy link
Member

This is Haskell not Python.

It's using python to build. The bot has a simple mind.

@fxcoudert
Copy link
Member

So this relies on Rosetta 2 to bootstrap, but the resulting build will not have any x86_64 files anymore, and can be run on a machine without Rosetta 2, is that correct?

If we could host somewhere an ARM toolchain to use as a bootstrap compiler, would it be possible to make this formula totally independent of Rosetta 2?

@hjelmn
Copy link
Contributor Author

hjelmn commented Dec 1, 2020

Yes. This uses rosetta to essentially build a bootstrap binary. That is then used like the x86_64 bootstrap binary that was downloaded from ghc. Let me see what the process is to build a bootstrap tarball.

@hjelmn
Copy link
Contributor Author

hjelmn commented Dec 1, 2020

Looks like it is make binary-dist. Will try that now with the ghc I have installed.

@hjelmn
Copy link
Contributor Author

hjelmn commented Dec 1, 2020

Ok, that worked. Now to find someplace to put this binary-dist tarball....

@gitfoxi
Copy link

gitfoxi commented Dec 14, 2020

Hi. I had GHC 7 on aarch64 sort of working on Linux a few years ago. I think the key is to use LLVM only. (Porting the x64 code generator seems hard and unnecessary. But I do want to avoid Rosetta. She's slowing down JVM and CLR so my guess is she won't be great for GHC either.) For homebrew this would mean depend on LLVM and use -fllvm when building GHC then always use -fllvm to build other software. But first I need to try it in a vanilla build.

@fishtreesugar
Copy link
Contributor

Hey @hjelmn, GHC 8.10.3 already released. Would you like to bump the GHC version to 8.10.3 instead of 8.10.2?

@fishtreesugar
Copy link
Contributor

Hey @hjelmn, GHC 8.10.3 already released. Would you like to bump the GHC version to 8.10.3 instead of 8.10.2?

Oh, I just found #65759, let's do this in #65759

@fishtreesugar
Copy link
Contributor

Official support related ticket for reference: Support for Apple ARM hardware

@hjelmn
Copy link
Contributor Author

hjelmn commented Dec 21, 2020

Yes. I will bump this to 8.10.3. The patch should be similar.

@carlocab
Copy link
Member

I think this should be ready soon: #67284

In which case you may wish to rebase this PR against master instead when that happens.

@fxcoudert
Copy link
Member

@hjelmn could you please rebase this on top of current master? ghc 8.10.3 is now in homebrew

@hjelmn
Copy link
Contributor Author

hjelmn commented Dec 30, 2020

Ok, will rebase it tonight.

@lqf96
Copy link

lqf96 commented Dec 31, 2020

It seems that for GHC 9 the GMP dependency may be optional... According to the release notes, GHC now has a native backend for big integer support...

@hjelmn hjelmn changed the title add support for Apple Silicon with ghc 8.10.2 add support for Apple Silicon with ghc 8.10.3 Dec 31, 2020
@hjelmn
Copy link
Contributor Author

hjelmn commented Dec 31, 2020

@lqf96 Good to hear. 9.x is the way to go long term with 8.10.x just a stop-gap until that is ready.

@hjelmn
Copy link
Contributor Author

hjelmn commented Dec 31, 2020

Ok, rebased. Re-installed from this formula and it appears to be working fine:

==> Installing ghc
==> Patching
==> Applying ghc-8.10.3.patch
patching file AArch64.hs
patching file aclocal.m4
patching file compiler/GHC/Platform/AArch64.hs
patching file compiler/GHC/Platform/Regs.hs
patching file compiler/cmm/PprC.hs
patching file compiler/codeGen/CodeGen/Platform/AArch64.hs
patching file compiler/ghc.cabal.in
patching file compiler/main/DriverPipeline.hs
patching file compiler/nativeGen/AsmCodeGen.hs
patching file compiler/nativeGen/RegAlloc/Graph/TrivColorable.hs
patching file compiler/nativeGen/RegAlloc/Linear/FreeRegs.hs
patching file compiler/nativeGen/RegAlloc/Linear/Main.hs
patching file compiler/nativeGen/TargetReg.hs
patching file config.guess
patching file configure
patching file includes/CodeGen.Platform.hs
patching file includes/rts/storage/GC.h
patching file includes/stg/Regs.h
patching file libraries/ghc-boot/GHC/Platform.hs
patching file libraries/ghci/GHCi/InfoTable.hsc
patching file libraries/integer-gmp/gmp/gmpsrc.patch
patching file llvm-targets
patching file rts/Adjustor.c
patching file rts/StgCRun.c
patching file rts/linker/elf_plt_aarch64.c
patching file rts/linker/elf_reloc.c
patching file rts/package.conf.in
patching file rts/rts.cabal.in
patching file rts/sm/Storage.c
patching file utils/llvm-targets/gen-data-layout.sh
==> ./configure --prefix=/private/tmp/ghc-20201230-50684-ufihmr/ghc-8.10.3/binary
==> make install
==> type -p clang
==> arch -x86_64 ./configure --prefix=/private/tmp/ghc-20201230-50684-ufihmr/ghc-8.10.3/binary-cross --target=arm64-apple-darwin --enable-unregisterised
==> make
==> make install
==> make distclean
==> ./configure --prefix=/opt/homebrew/Cellar/ghc/8.10.3_2 --enable-unregisterised
==> make
==> make install
==> /opt/homebrew/Cellar/ghc/8.10.3_2/bin/ghc-pkg recache
🍺  /opt/homebrew/Cellar/ghc/8.10.3_2: 6,901 files, 2.0GB, built in 141 minutes 52 seconds

Formula/ghc.rb Outdated Show resolved Hide resolved
@hjelmn
Copy link
Contributor Author

hjelmn commented Dec 31, 2020

If this is acceptable I will open a PR to put the patch in homebrew-patches.

@hjelmn
Copy link
Contributor Author

hjelmn commented Dec 31, 2020

Bumped the revision to 2. Let me know if that makes sense.

@carlocab
Copy link
Member

I think submitting your patch to formula-patches should be fine. In your PR, include an explanation of where the patch came from (i.e. backported from master), as you'll likely be asked why it hasn't been upstreamed if you don't.

Formula/ghc.rb Outdated Show resolved Hide resolved
@fxcoudert fxcoudert added the CI-force-arm [DEPRECATED] Don't pass --skip-unbottled-arm to brew test-bot. label Dec 31, 2020
@angerman
Copy link
Contributor

angerman commented Jan 7, 2021

NB: We do have some M1s, and hopefully they will be in the CI pipeline for GHC soonish; at which point we should have much improved testing.

@hjelmn
Copy link
Contributor Author

hjelmn commented Jan 7, 2021

@angerman Awesome! Also please take a look at the libffi patch I posted for formula-patches. Seems Apple has some fixes. With that patch the ffi tests pass.

@fxcoudert
Copy link
Member

@hjelmn I'm willing to build (and host) the bootstrap compiler whenever you think you're ready. Is ghc 8.10.3 + your patch at https://gist.githubusercontent.com/hjelmn/49f70df40da0399006031d854f73473e/raw/601bed7124f0fb9787d94806eab61645b85dab0a/ghc-8.10.3.patch a suitable starting place, or should I wait for more patches? (It's unclear to me from the discussion above)

What should I do to generate this bootstrap binary? Can I use the formula in its current state?

@hjelmn
Copy link
Contributor Author

hjelmn commented Jan 8, 2021

@fxcoudert I have an improved patch and formula. Let me get those up this evening and I can provide instructions on how to build a bootstrap binary.

@carlocab
Copy link
Member

Any progress on this, @hjelmn? I don't mean to rush you; just checking in.

@angerman
Copy link
Contributor

Any progress on this, @hjelmn? I don't mean to rush you; just checking in.

I'm afraid @hjelmn might not be able to get this fixed fully. There is some fundamental design issues in GHC that prevent foreign calls to work properly on arm64. This is due to the arm64 calling convention (e.g. aarch64 on darwin), where argumetns need to be packed by sized on the stack when spilled, and also extended as needed. GHC internally treats every element at the FFI boundary as Word, which works with calling conventions on x86, and linux, but falls apart on arm64. We've fixed this in GHC HEAD, and it should be fixed in 9.2. This came actually up during the development of a native code gen (which I'll try to get into 9.2 as well). I'm currently trying to devise some clever band-aid for 8.10 and 9.0 for native arm64 compilers.

@carlocab
Copy link
Member

GHC internally treats every element at the FFI boundary as Word, which works with calling conventions on x86, and linux, but falls apart on arm64.

This actually sounds eerily like other ARM-related bugs we've seen.

I'm currently trying to devise some clever band-aid for 8.10 and 9.0 for native arm64 compilers.

That would be nice since 9.2 sounds like a long way away. Thanks for the update, @angerman.

@hjelmn
Copy link
Contributor Author

hjelmn commented Jan 26, 2021

@angerman Yup. I can get a good chuck of the test suite to pass but the failures have prevented me from moving forward. Can you point me at the 9.2 commit which fixes the issue? I may be able to find some time to see about working it into 8.10 if that helps.

@angerman
Copy link
Contributor

@carlocab, you can find more info on the abi and calling conventions here: https://developer.apple.com/documentation/xcode/writing_arm64_code_for_apple_platforms

@hjelmn This commit: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/4390, adds the required logic, however back porting is not an option, as it has severe ripple effects on libraries. E.g. we'd need to update each library to accommodate this. A lot of of this is happening in HEAD, but the same reason this won't be in 9.0.

I'm currently trying to add meta data through the calling convetion from the foreign import sites all the way to the codegen, so the codegen can do the appropriate from/to word conversions. Some of it is in here: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/4795. But the major change is still missing. I used to have a temp fix in the arm64 branch, but that seems to miss some logic in 8.10 and was very hacky.

My plan is to fix this properly for 8.10, but I'm afraid it might take a bit more time, also full turn-around for a full build on arm64 is ~90min, due to the llvm backend in GHC being quite slow. The NCG will be about 3times faster, e.g. taking ~30min for a full build.

@carlocab it is very important on arm64 to have correct header files, that match the types of the implementations, otherwise the calling convention breaks. Usually clang will warn about this as well a lot more aggressively on arm64.

@angerman
Copy link
Contributor

@hjelmn if you feel like using https://gitlab.haskell.org/ghc/ghc/-/merge_requests/4795 to extract a patch, you might get lucky now. Test suite failures are down to three, that are mildly concerning, but other test suite failures that remain are either expected (tests is broken for arm64) or known to be unfixable as of right now.

@SeekingMeaning SeekingMeaning removed the python Python use is a significant feature of the PR or issue label Jan 29, 2021
@BrewTestBot BrewTestBot added the no ARM bottle Formula has no ARM bottle label Jan 29, 2021
@hjelmn
Copy link
Contributor Author

hjelmn commented Jan 29, 2021

@angerman Ok, will try to make a patch from that tonight.

@angerman
Copy link
Contributor

@hjelmn, @SeekingMeaning

There is no native code generator for arm64 at this time. The resulting binaries will likely be slower than if there was a native code generator.
(this, is from 1faf91d)

this statement is not correct. The NCG is a lot faster during compilation, but not necessary during runtime. Runtime results can vary a lot depending on the program. Compilation time however is always worse with the llvm codegen.

@hjelmn hjelmn force-pushed the ghc_apple_silicon branch from 1128410 to 1fb9e34 Compare February 1, 2021 15:34
This PR adds support to ghc.rb for Apple Silicon Macs. This PR removes the
external build of gmp in favor of using the in-tree one as the in-tree one
is statically linked these days (confirmed no external gmp references). The
version in-tree is 6.1.2 which does not build on Apple Silicon so it is
replaced by the formula with 6.2.1 which does.

In order to get a full install with a working arm64 binary we currently need
to build ghc in three steps:

 1) Install the ghc binaries for x86_64 into a temporary directory (this is
    also done for x86_64).

 2) Build a cross-compiler for arm64 using the x86_64 binary to bootstrap.

 3) Use either the x86_64 binaries (on x86_64) or the cross-compiler to build
    a native ghc for the system.

This PR relies on an extensive patch based on a PR to ghc 8.10.x. These
patches add support for arm64-apple-darwin and correct some build system
confusion between the ghc target name (aarch64-apple-darwin) and the llvm
target name (arm64-apple-darwin).

Some caveats:

 - This build takes awhile (over 2 hours). This is a significant build time
   for an M1 system given it is roughly twice as fast as a Core i9.

 - We can probably shorten the build time by using integer-simple and
   disabling other libraries but this formula is a stop-gap measure until
   upstream releases official support (ghc 9.x series).

 - There is no native code generator for arm64 at this time. The resulting
   binaries will likely be slower than if there was a native code generator.

Patch updated by @SeekingMeaning

Signed-off-by: Nathan Hjelm <[email protected]>
@hjelmn hjelmn force-pushed the ghc_apple_silicon branch from 1fb9e34 to bf1112b Compare February 1, 2021 15:36
@hjelmn
Copy link
Contributor Author

hjelmn commented Feb 1, 2021

Ok, looks like we now have a good patch thanks to @angerman and @SeekingMeaning. Can you use that patch to build the binary dist? Once that is done I can update this PR to use that for ARM64 and remove the cross-compiling step. I have been building a cross-compiler using the steps in this PR then using that to actually build ghc.

@angerman Is llvm 9 required or can a newer version be used? I have gotten it to work with the system compiler but then we don't have opt or llc.

@carlocab
Copy link
Member

carlocab commented Feb 1, 2021

Can you use that patch to build the binary dist? Once that is done I can update this PR to use that for ARM64 and remove the cross-compiling step.

CC @fxcoudert

@hjelmn
Copy link
Contributor Author

hjelmn commented Feb 1, 2021

I need to update the patch one more time. Looks like @SeekingMeaning forgot to include changes to configure. I will run autoreconf and generate a new patch.

To keep it clean so we have the original patch I will just append the configure changes to the existing patch.

@angerman
Copy link
Contributor

angerman commented Feb 2, 2021

@angerman Is llvm 9 required or can a newer version be used? I have gotten it to work with the system compiler but then we don't have opt or llc.

You can use llvm11, in fact that's what I've been doing all along. The warning about llvm9 is due to ghc trying to be very conservative about making promises of fitness. However the test-suite I ran was against llvm11, so I think we are fairly good with 11.

@hjelmn
Copy link
Contributor Author

hjelmn commented Feb 6, 2021

With the patch @SeekingMeaning uploaded I keep running into a cross-compile issue I used to be able to work around. For some reason the stage1 is generating intel assembly. The way I had worked around this was to ensure configure was run as an x86 binary. That is no longer working. What I may do is try 8.10 head + @angerman's patch and see what I get. I am working on the cross-compile step to generate a binary tarball. Its quite possible if I use to 8.10.3 build I already have it may just work.

@BrewTestBot
Copy link
Member

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@BrewTestBot BrewTestBot added the stale No recent activity label Feb 28, 2021
@BrewTestBot BrewTestBot closed this Mar 7, 2021
@github-actions github-actions bot added the outdated PR was locked due to age label Apr 6, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI-force-arm [DEPRECATED] Don't pass --skip-unbottled-arm to brew test-bot. no ARM bottle Formula has no ARM bottle outdated PR was locked due to age stale No recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants