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

Use the GNU Gold or LLVM LLD linker if available #475

Merged
merged 1 commit into from
Jan 18, 2018

Conversation

nilsdeppe
Copy link
Member

Proposed changes

Use the GNU Gold and LLVM LLD linkers if they're available.

These linkers are quite a bit faster than LD. On my machine I get:
ld: 11.7 s
ld.gold: 6.5 s
ld.lld: 4.9 s

when linking RunTests

Types of changes:

  • Bugfix
  • New feature

Component:

  • Code
  • Documentation
  • Build system
  • Continuous integration

Code review checklist

  • Follows code review guidelines
  • Code has documentation and unit tests
  • Private member variables have a trailing underscore
  • Do not use Hungarian notation, e.g. double* pd_blah is bad
  • Header order:
    1. hpp corresponding to cpp (only in cpp files)
    2. Blank line (only in cpp files)
    3. STL and externals (in alphabetical order)
    4. Blank line
    5. SpECTRE includes (in alphabetical order)
  • File lists in CMake are alphabetical
  • Correct noexcept specification for functions (if unsure, mark noexcept)
  • Mark objects const whenever possible
  • Almost always auto, except with expression templates, i.e. DataVector
  • All commits for performance changes provide quantitative evidence and the tests used to obtain said evidence.
  • Make sure error messages are helpful, e.g. "The number of grid points in the matrix 'F' is not the same as the number of grid points in the determinant."
  • Prefix commits addressing PR requests with fixup

@codecov-io
Copy link

codecov-io commented Jan 13, 2018

Codecov Report

Merging #475 into develop will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #475      +/-   ##
===========================================
+ Coverage    98.78%   98.78%   +<.01%     
===========================================
  Files          212      213       +1     
  Lines         7983     7988       +5     
===========================================
+ Hits          7886     7891       +5     
  Misses          97       97
Impacted Files Coverage Δ
src/Utilities/TaggedTuple.hpp 99.25% <0%> (-0.03%) ⬇️
src/Parallel/Printf.hpp 100% <0%> (ø) ⬆️
src/ControlSystem/PiecewisePolynomial.hpp 100% <0%> (ø) ⬆️
src/DataStructures/Tensor/Tensor.hpp 100% <0%> (ø) ⬆️
src/ControlSystem/FunctionOfTime.hpp 100% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e62445c...a2e18a4. Read the comment docs.

Copy link
Member

@wthrowe wthrowe left a comment

Choose a reason for hiding this comment

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

There needs to be a way for the user to override this choice. I've used systems where ld.gold would successfully link binaries but those binaries wouldn't run.

These linkers are quite a bit faster than LD. On my machine I get:
ld: 11.7 s
ld.gold: 6.5 s
ld.lld: 4.9 s

when linking RunTests
@nilsdeppe
Copy link
Member Author

I've updated the commit. Given that it's a single file I just squashed since that's probably easier to read than a diff. Let me know if you think the option name should be changed, I went with what the underlying flag to the compiler is.

@nilsdeppe nilsdeppe merged commit 756c8e4 into sxs-collaboration:develop Jan 18, 2018
@nilsdeppe nilsdeppe deleted the use_faster_linker branch January 24, 2018 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants