Skip to content
This repository has been archived by the owner on Aug 18, 2020. It is now read-only.

[CSL-1002] Link project with gold linker #193

Merged
merged 1 commit into from
Apr 14, 2017
Merged

[CSL-1002] Link project with gold linker #193

merged 1 commit into from
Apr 14, 2017

Conversation

chshersh
Copy link
Contributor

@chshersh chshersh commented Apr 6, 2017

This PR uses gold linker for cardano-sl (should speed up build time).

Details in the answer to this question:
http://stackoverflow.com/questions/43243322/how-to-link-with-the-gnu-gold-linker-instead-of-ld-in-haskell/43243323#43243323

@mention-bot
Copy link

@chshersh, thanks for your PR! By analyzing the history of the files in this pull request, we identified @neongreen, @gromakovsky and @flyingleafe to be potential reviewers.

@neongreen
Copy link
Contributor

  • does it work on macOS?
  • does it work on Windows?

@gromakovsky
Copy link
Contributor

We have CI to answer this question 😉 .
I will test it on MacOS by myself though.

@gromakovsky
Copy link
Contributor

Doesn't work on Windows.

@neongreen
Copy link
Contributor

Beda.

@chshersh can you use cabal conditionals to disable it on Windows?

@gromakovsky so does it work on macOS or not? Travis tests on macOS, right?

@gromakovsky
Copy link
Contributor

Yes, Travis does test MacOS, but builds failed.

@gromakovsky
Copy link
Contributor

clang-3.7: error: invalid linker name in argument '-fuse-ld=gold' (in travis, macos)

@chshersh
Copy link
Contributor Author

chshersh commented Apr 6, 2017

It was expected that won't work on windows. Okay, I'll add cabal options to enable this only on linux.

@chshersh
Copy link
Contributor Author

chshersh commented Apr 7, 2017

@domenkozar
Copy link
Contributor

You'd use lld on macOS and gold on Linux.

@chshersh chshersh changed the title Link project with gold linker [CSL-1002] Link project with gold linker or lld Apr 11, 2017
@chshersh
Copy link
Contributor Author

I'm not sure it works properly on macOS because probably project should be configured in different way for lld to work according to this example:
https://github.com/nh2/link-with-lld-example/blob/master/link-with-lld-example.cabal

@gromakovsky
Copy link
Contributor

MacOS build in CI: https://travis-ci.org/input-output-hk/cardano-sl/jobs/221020730

clang-3.7: error: unsupported option '--cpp'

`clang' failed in phase `C pre-processor'. (Exit code: 1)

Copy link
Contributor

@gromakovsky gromakovsky left a comment

Choose a reason for hiding this comment

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

Seems to be broken on MacOS which is bad I think.

@gromakovsky
Copy link
Contributor

Same error on my macbook, btw (I don't use nix).

@chshersh
Copy link
Contributor Author

I'm not a macOS expert and I don't even have macOS :(
It works now on windows and linux. So somebody with mac should try to investigate problems and fix this.

@neongreen
Copy link
Contributor

@chshersh can you try without "-pgmP clang"?

@chshersh
Copy link
Contributor Author

@neongreen What do you mean by saying _ try without "-pgmP clang"_? Just remove this code, commit and wait untill somebody with mac build this?

@neongreen
Copy link
Contributor

Remove, commit and wait until CI builds this

@neongreen
Copy link
Contributor

I'll do it by myself actually

@neongreen
Copy link
Contributor

@domenkozar how can I install lld conditionally on macOS? There's stdenv.isLinux but I haven't found a corresponding var for macOS.

Also, blocked by NixOS/nixpkgs#24744.

@neongreen
Copy link
Contributor

Niklas is going to fix NixOS/nixpkgs#24744, but while we're waiting it'd be nice to get just gold on Linux. @chshersh can you split lld into a separate PR?

@domenkozar
Copy link
Contributor

stdenv.isDarwin. I think this PR is premature optimization for what we're doing since compilation itself is long including dependencies. This is why @jmitchell is working on stack2nix

@neongreen
Copy link
Contributor

I think this PR is premature optimization for what we're doing since compilation itself is long including dependencies.

It's not for CI, it's for developers.

@domenkozar
Copy link
Contributor

Ah, excuse my ignorance. That does make sense.

Ideally you could use stack to only build one executable instead of all of them, but that's waiting on next Cabal release.

Since lld seems like a long shot, we might give it a try with LLVM 4.0 and gold:

@chshersh
Copy link
Contributor Author

chshersh commented Apr 13, 2017

can you split lld into a separate PR?

Yes, I can. Will to it today then.

@chshersh chshersh changed the title [CSL-1002] Link project with gold linker or lld [CSL-1002] Link project with gold linker Apr 14, 2017
@chshersh
Copy link
Contributor Author

@neongreen I'll open PR with lld linker after this PR is merged to avoid further conflicts.

@gromakovsky gromakovsky merged commit 4e14b2e into master Apr 14, 2017
@gromakovsky gromakovsky deleted the gold-linker branch April 14, 2017 18:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants