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 linker optimizations on Linux #10620

Closed
wants to merge 1 commit into from
Closed

Use linker optimizations on Linux #10620

wants to merge 1 commit into from

Conversation

emberian
Copy link
Member

No description provided.

@@ -1097,6 +1097,10 @@ pub fn link_args(sess: Session,
// which lives in libm. Similar to above, on some linuxes we
// have to be explicit about linking to it. See #2510
args.push(~"-lm");

// The GNU linker supports optimizations. Use them. gc-sections removes metadata and
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you phrase this as GNU-style linkers? It applies to ld.bfd, ld.gold (from binutils) and also lld -flavor gnu.

@alexcrichton
Copy link
Member

I'm a little uncomfortable with this. There is no mention of any concrete improvement in this pull request or the comments, so it's unclear to me why we want this by default. Additionally, from a man page I found:

If level is a numeric values greater than zero ld optimizes the output. This might take significantly longer and therefore probably should only be enabled for the final binary. At the moment this option only affects ELF shared library generation. Future releases of the linker may make more use of this option. Also currently there is no difference in the linker's behaviour for different non-zero values of this option. Again this may change with future releases.

Which sounds like it's recommending that we do not use this for libraries, but only for binaries. It appears that this pull request is unconditionally running it with optimizations.

What's the main reason for adding this? I don't want to just run optimizations to run optimizations. I just want to make sure that we have a clear benefit from this. Additionally, does clang do this at all? Is there precedent for this?

// GNU-style linkers will use this to omit linking to libraries which don't actually fulfill
// any relocations, but only for libraries which follow this flag. Thus, use it before
// specifing libraries to link to.
args.push(~"-Wl,--as-needed");
Copy link
Member

Choose a reason for hiding this comment

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

This is a subtle change in linking that no one is able to change without recompiling the compiler. What are the benefits of doing this? The quote I found from the manpage was:

This option affects ELF DT_NEEDED tags for dynamic libraries mentioned on the command line after the --as-needed option. Normally the linker will add a DT_NEEDED tag for each dynamic library mentioned on the command line, regardless of whether the library is actually needed or not. --as-needed causes a DT_NEEDED tag to only be emitted for a library that satisfies an undefined symbol reference from a regular object file or, if the library is not found in the DT_NEEDED lists of other libraries linked up to that point, an undefined symbol reference from another dynamic library. --no-as-needed restores the default behaviour.

Which sounds like a library may not link to a native dynamic library even if I ask the compiler to do so. I don't really know of a use case for doing that, but I don't want to prohibit that from happening just because I don't understand the use case.

Copy link
Contributor

Choose a reason for hiding this comment

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

It avoids linking against shared objects on platforms where they don't provide any of the symbols used by the program. For example, clock_gettime was moved from librt.so to libc.so in modern glibc versions but the crate is likely to still specify -lrt if it needs it. I can't think of a valid reason to override it. It really only needs to be a flag for backwards compatibility with broken build systems not linking in the right order.

@thestinger
Copy link
Contributor

@alexcrichton: The precedent for this is every widely used Linux distribution using these flags to build packages. Ubuntu, Debian, Fedora, Arch Linux, OpenSUSE, etc. all use these (and most likely use --sort-common too).

It currently only optimizes dynamic libraries and not binaries, but it will likely optimize binaries in the future. There is no harm in telling it to optimize a binary and it will transparently start doing it when it's improved in the future.

All this part is saying is that there's currently only a binary choice between optimizing and not optimizing, with no distinction between numeric values:

Also currently there is no difference in the linker's behaviour for different non-zero values of this option. Again this may change with future releases.

@emberian
Copy link
Member Author

I don't particularly care about this PR, just cleaning up #6190 from triage.

@emberian
Copy link
Member Author

(this week, three months later, same thing...)

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