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: really use ld.gold #80466

Merged
merged 1 commit into from
Feb 27, 2020
Merged

Conversation

adamse
Copy link
Contributor

@adamse adamse commented Feb 18, 2020

Motivation for this change

The changes to use ld.gold in other scenarios than aarch32 had not been propagated fully. This change makes sure ghc will call gcc with -fuse-ld=gold.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested review from peti and kosmikus February 18, 2020 19:45
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Feb 18, 2020
@cdepillabout
Copy link
Member

@adamse Thanks for taking the time to send a PR for this.

Could explain a little more what this does?

From what I can tell, the original code had attempted to get GHC to use ld.gold on linux when using GCC (well, non-llvm), but it didn't correctly pass things like CFLAGS=-fuse-ld=gold. So I'm guessing the result was that GHC wasn't actually using ld.gold?

If this is correct, then it seems like it will be a pretty big change if we switch the linker we use for all our haskell packages? Did you try recompiling a bunch of the Haskell package set with this change? Did anything break?

Also, if I wanted to test this, what's the best way for me to test that GHC is actually using ld.gold? That is to say, how do I know that this PR is doing what is says it is supposed to be doing?

@adamse
Copy link
Contributor Author

adamse commented Feb 19, 2020

The end result of this PR is that ghc will pass -fuse-ld=gold to gcc when ghc calls gcc for linking purposes.

To confirm that ld.gold is used we can look at ghc --info, it should contain:

 ,("C compiler link flags","-fuse-ld=gold ")

after this change. We can also confirm it by looking at the ghc -v3 output (which I've done for this change for the 8.8.2 version). (I also spent some time confusing myself by looking at the nixpkgs master version of 8.8.2 and finding it did indeed not have nor use -fuse-ld=gold.)

I've built the lens library (with dependencies) with 8.8.2.

I've also fired off a nixpkgs-review pr 80466 but I'm afraid I might run out of diskspace before any meaningful results are obtained. Can I use it more effectively?

@cdepillabout
Copy link
Member

@adamse Thanks for the explanation.

That all seems pretty reasonable.

This PR looks good to me, and the testing you've done sounds pretty good. However, it would be nice to have someone like @matthewbauer / @Ericson2314 / @peti take a look at this as well.

I guess it is entirely possible ld.gold was used at some point, and it was reverted on purpose, as opposed to it just being an oversight. One of the three above would probably know about it if that is the case.

@pepeiborra
Copy link
Contributor

Looking forward to this

@domenkozar domenkozar changed the base branch from master to haskell-updates February 27, 2020 07:58
@domenkozar domenkozar merged commit e84ac60 into NixOS:haskell-updates Feb 27, 2020
@ggreif
Copy link
Contributor

ggreif commented Feb 27, 2020

@adamse In the meanwhile 8.8.3 has landed (#80975) in haskell-updates, it would need a similar treatment as 8.8.2 here.

ggreif added a commit to ggreif/nixpkgs that referenced this pull request Feb 28, 2020
domenkozar added a commit that referenced this pull request Feb 28, 2020
ghc-8.8.3: really use ld.gold (port #80466)
@adamse
Copy link
Contributor Author

adamse commented Feb 29, 2020

Thanks @ggreif, I've created #81357 to address that.

domenkozar pushed a commit that referenced this pull request Mar 23, 2020
(cherry picked from commit ff6aeef)
Signed-off-by: Domen Kožar <[email protected]>
@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: haskell 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants