Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use ld.gold in dev shells #1868

Merged
merged 10 commits into from
Mar 7, 2023
Merged

Conversation

TeofilC
Copy link
Contributor

@TeofilC TeofilC commented Feb 28, 2023

Previously we built GHC with ld.gold for aarch32, and forced use of ld.gold in the component builder for non-android linux. Though this meant that ld.gold wasn't used in dev shells on non-android linux as that uses a different code-path.

This patch unifies all of this logic in one place: in the GHC build script.
If GHC is fed LD as being ld.gold then it will write that into its settings and try to use that at runtime.

So, we also build GHC with ld.gold on non-android linux, meaning that it uses that both in component builds and dev shells.


Unfortunately this will lead to mass rebuilds. I experimented with just duplicating the component logic in the ghcWrapper used in dev shells, but this led to mass rebuilds anyway, so, I think we might as well do the more principled thing.


Resolves #1866

@michaelpj
Copy link
Collaborator

Did you manage to build a shell with this? Do you get the link-time speedups we'd expect?

@TeofilC
Copy link
Contributor Author

TeofilC commented Feb 28, 2023

@michaelpj I've tested with GHC-9.2.6 on x86_64-linux and as far as I can tell it's using ld.gold. I'll try to test with a hadrian built GHC as well just to be safe

@TeofilC
Copy link
Contributor Author

TeofilC commented Feb 28, 2023

I tested the difference by changing the Main module for a large exe and running stack build.
It took 30s to recompile/link with gold and 45s with standard ld.

@michaelpj
Copy link
Collaborator

Great stuff.

@TeofilC
Copy link
Contributor Author

TeofilC commented Feb 28, 2023

Checked that 9.4.4 also has the ,("C compiler link flags","-fuse-ld=gold ") line in ghc --info

@TeofilC
Copy link
Contributor Author

TeofilC commented Feb 28, 2023

So, we are definitely passing -fuse-ld=gold to gcc but I'm a bit worried that the other calls to the linker might be still going via ld.bfd.

I've found this comment: https://gitlab.haskell.org/ghc/ghc/-/issues/22550#note_466656
So I'll add a commit that also adds that to the configure flags.

@TeofilC
Copy link
Contributor Author

TeofilC commented Mar 1, 2023

That built overnight and now I also have this line in ghc --info ,("Merge objects command","ld.gold")

@angerman
Copy link
Collaborator

angerman commented Mar 6, 2023

@TeofilC is this ready to be merged? We do squash merge; let me know if you have any concerns with that.

@TeofilC
Copy link
Contributor Author

TeofilC commented Mar 6, 2023

Thanks for checking @angerman . Yeah I think this is ready. Squash merge sounds great

@angerman angerman merged commit 5240aeb into input-output-hk:master Mar 7, 2023
Comment on lines +72 to +73
(stdenv.targetPlatform.isLinux && !stdenv.targetPlatform.isAndroid && !stdenv.targetPlatform.isMusl)
|| stdenv.targetPlatform.isAarch32
Copy link
Collaborator

Choose a reason for hiding this comment

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

After looking closer at this, I believe this is wrong. We do not unconditionally enable this in aarch32.
I believe the || stdenv.targetPlatform.isAarch32 should go.

@thomasjm
Copy link
Contributor

Hi, this commit has actually broken my static builds with musl. This is on x64 Linux, so the isAarch32 issue isn't relevant. I now get linker errors like the following:

       > Preprocessing executable 'codedown-editor' for codedown-editor-0.1.0.0..
       > Building executable 'codedown-editor' for codedown-editor-0.1.0.0..
       > [1 of 2] Compiling Main             ( main/Main.hs, dist/build/codedown-editor/codedown-editor-tmp/Main.o, dist/build/codedown-editor/codedown-editor-tmp/Main.dyn_o )
       > [2 of 2] Compiling Paths_codedown_editor ( dist/build/codedown-editor/autogen/Paths_codedown_editor.hs, dist/build/codedown-editor/codedown-editor-tmp/Paths_codedown_editor.o, dist/build/codedown-editor/codedown-editor-tmp/Paths_codedown_editor.dyn_o )
       > Linking dist/build/codedown-editor/codedown-editor ...
       > /nix/store/bz5jhvvyxkhngylsipv4c7v2arb4hmpi-binutils-x86_64-unknown-linux-musl-2.39/bin/ld: attempted static link of dynamic object `/nix/store/mg2frc8mybn9ga75w4mry7pmi6v6phmw-gcc-x86_64-unknown-linux-musl-11.3.0-lib/lib/libstdc++.so'
       > /nix/store/bz5jhvvyxkhngylsipv4c7v2arb4hmpi-binutils-x86_64-unknown-linux-musl-2.39/bin/ld: attempted static link of dynamic object `/nix/store/ilfgsdvk8srxfh73w8yg45b9bcscsvhj-musl-x86_64-unknown-linux-musl-1.2.3/lib/libc.so'
       > /nix/store/bz5jhvvyxkhngylsipv4c7v2arb4hmpi-binutils-x86_64-unknown-linux-musl-2.39/bin/ld: attempted static link of dynamic object `/nix/store/ilfgsdvk8srxfh73w8yg45b9bcscsvhj-musl-x86_64-unknown-linux-musl-1.2.3/lib/libc.so'
       > collect2: error: ld returned 1 exit status
       > `g++' failed in phase `Linker'. (Exit code: 1)

@thomasjm
Copy link
Contributor

It appears that my static executable build was using ld.gold prior to this commit, as I see arguments like this in the GHC configure flags:

--with-ld=x86_64-unknown-linux-musl-ld.gold --ghc-option=-optl-fuse-ld=gold --ld-option=-fuse-ld=gold

I'm confused why this PR disables ld.gold for musl when the linked Bugzilla issue says it was fixed in 2017?

Is there a way for me to pass useLdGold=true manually in my stackProject call?

@TeofilC
Copy link
Contributor Author

TeofilC commented Mar 22, 2023

Hi sorry that this broke stuff for you @thomasjm ! I think I was overly cautious about not having ld.gold for musl. I'll make an MR to enable it.

Until then I think you can workaround it by using a module like:

let
  compiler-nix-name = "ghc927"
in {
  ghc.packages = pkgs.haskell-nix.compiler."${compiler-nix-name}".override { useLdGold = true};
}

I think there may be a more idiomatic way of doing this but I've done stuff similar to this in the past.

@thomasjm
Copy link
Contributor

Thanks for the module option, I got it to accept the following:

ghc.package = pkgs.buildPackages.haskell-nix.compiler.ghc925.override { useLdGold = true; };

Unfortunately, this crashes with a scary looking segfault when trying to build GHC:

error: builder for '/nix/store/5aj8a7hrpcyvicfa9cmc32bvgwdz59xp-x86_64-unknown-linux-musl-ghc-9.2.5.drv' failed with exit code 139;                                                                                 
       last 10 log lines:                                                                                                                                                                                           
       > "inplace/bin/ghc-cabal" register libraries/ghci dist-install "/build/ghc-9.2.5/inplace/bin/ghc-stage1" "/build/ghc-9.2.5/utils/ghc-pkg/dist/build/tmp/ghc-pkg" "/nix/store/m79nmslgk5vk4zp0gi99wbh1ac24lz5w-x86_64-unknown-linux-musl-ghc-9.2.5/lib/x86_64-unknown-linux-musl-ghc-9.2.5" '' '/nix/store/m79nmslgk5vk4zp0gi99wbh1ac24lz5w-x86_64-unknown-linux-musl-ghc-9.2.5' '/nix/store/m79nmslgk5vk4zp0gi99wbh1ac24lz5w-x86_6
4-unknown-linux-musl-ghc-9.2.5/lib/x86_64-unknown-linux-musl-ghc-9.2.5' '/nix/store/hanp7rdwa6sgynaz71l5l3qf2kckrjc9-x86_64-unknown-linux-musl-ghc-9.2.5-doc/share/doc/x86_64-unknown-linux-musl-ghc/html/libraries' NO                                                                                                                                                                                                                  
       > Registering library for ghci-9.2.5..                                                                                                                                                                       
       > "inplace/bin/ghc-cabal" register libraries/libiserv dist-install "/build/ghc-9.2.5/inplace/bin/ghc-stage1" "/build/ghc-9.2.5/utils/ghc-pkg/dist/build/tmp/ghc-pkg" "/nix/store/m79nmslgk5vk4zp0gi99wbh1ac24lz5w-x86_64-unknown-linux-musl-ghc-9.2.5/lib/x86_64-unknown-linux-musl-ghc-9.2.5" '' '/nix/store/m79nmslgk5vk4zp0gi99wbh1ac24lz5w-x86_64-unknown-linux-musl-ghc-9.2.5' '/nix/store/m79nmslgk5vk4zp0gi99wbh1ac24lz5w-x
86_64-unknown-linux-musl-ghc-9.2.5/lib/x86_64-unknown-linux-musl-ghc-9.2.5' '/nix/store/hanp7rdwa6sgynaz71l5l3qf2kckrjc9-x86_64-unknown-linux-musl-ghc-9.2.5-doc/share/doc/x86_64-unknown-linux-musl-ghc/html/libraries' NO                                                                                                                                                                                                              
       > Registering library for libiserv-9.2.5..                                                                                                                                                                   
       > "inplace/bin/ghc-cabal" register compiler stage2 "/build/ghc-9.2.5/inplace/bin/ghc-stage1" "/build/ghc-9.2.5/utils/ghc-pkg/dist/build/tmp/ghc-pkg" "/nix/store/m79nmslgk5vk4zp0gi99wbh1ac24lz5w-x86_64-unknown-linux-musl-ghc-9.2.5/lib/x86_64-unknown-linux-musl-ghc-9.2.5" '' '/nix/store/m79nmslgk5vk4zp0gi99wbh1ac24lz5w-x86_64-unknown-linux-musl-ghc-9.2.5' '/nix/store/m79nmslgk5vk4zp0gi99wbh1ac24lz5w-x86_64-unknown-li
nux-musl-ghc-9.2.5/lib/x86_64-unknown-linux-musl-ghc-9.2.5' '/nix/store/hanp7rdwa6sgynaz71l5l3qf2kckrjc9-x86_64-unknown-linux-musl-ghc-9.2.5-doc/share/doc/x86_64-unknown-linux-musl-ghc/html/libraries' NO         
       > Registering library for ghc-9.2.5..                                                                                                                                                                        
       > for f in '/nix/store/m79nmslgk5vk4zp0gi99wbh1ac24lz5w-x86_64-unknown-linux-musl-ghc-9.2.5/lib/x86_64-unknown-linux-musl-ghc-9.2.5/package.conf.d'/*; do create () { touch "$1" && chmod 644 "$1" ; } && create "$f"; done                                                                                                                                                                                                       
       > "/build/ghc-9.2.5/utils/ghc-pkg/dist/build/tmp/ghc-pkg" --global-package-db "/nix/store/m79nmslgk5vk4zp0gi99wbh1ac24lz5w-x86_64-unknown-linux-musl-ghc-9.2.5/lib/x86_64-unknown-linux-musl-ghc-9.2.5/package.conf.d" recache                                                                                                                                                                                                    
       > /nix/store/a7gvj343m05j2s32xcnwr35v31ynlypr-coreutils-9.1/bin/install -c -m 755 -d "/nix/store/m79nmslgk5vk4zp0gi99wbh1ac24lz5w-x86_64-unknown-linux-musl-ghc-9.2.5/bin"                                   
       > /nix/store/kmfaajdpyyyg319vfqni5jm9wkxjmf73-stdenv-linux/setup: line 89: 492870 Segmentation fault      "$out/bin/x86_64-unknown-linux-musl-ghc-pkg" recache                                               
       For full logs, run 'nix log /nix/store/5aj8a7hrpcyvicfa9cmc32bvgwdz59xp-x86_64-unknown-linux-musl-ghc-9.2.5.drv'.

AFAICT musl support has been working fine and there's no reason to disable it, so a PR to bring back the old behavior seems like the right thing to me.

@TeofilC
Copy link
Contributor Author

TeofilC commented Mar 22, 2023

I've made a branch that removes the musl special casing here: https://github.com/tracsis/haskell.nix/tree/use-ld-gold-musl

I'll try to test this locally before opening an MR, and see if that fixes things

@TeofilC
Copy link
Contributor Author

TeofilC commented Mar 22, 2023

Are you going via pkgsMusl or pkgsStatic?

@thomasjm
Copy link
Contributor

I use pkgsCross.musl64, I'm not familiar with the other ones.

@TeofilC TeofilC mentioned this pull request Mar 22, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use ld.gold in dev shells
4 participants