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

RFC: Split dynamic linker representations from Compiler representations #5148

Closed
wants to merge 9 commits into from

Conversation

dcbaker
Copy link
Member

@dcbaker dcbaker commented Mar 25, 2019

This is the start of the work for splitting the representation of the compiler and the dynamic linker into separate classes. The goal of this is ultimately to allow us to use the correct linker in all cases, instead of assuming that Clang uses gnu-ld, or other cases.

This is currently not ready to merge. There are still a couple of regressions on Linux I need to fix, as well as a host of compilers that I don't have access to that need to be updated and tested, including PGI, flang, elbrus, the sun fortran compiler, pathscale, swift, the arm compilers, and ccrx. I also haven't tested this on Windows or MacOS yet. While I think this is the right set of changes for an initial pass of the idea, I'd like to make a couple more changes, but in separate series:

  • Break the DynamicLinker out of the compiler as a peer object
  • Add a generic way to change the linker, such as via an LD field in cross/native files
  • Detect the linker in use and use the correct representation, like we do for compilers.

As long as this looks okay to people my next steps will be to test/fix windows and macos, and start pinging people who've added support for other compiler families to help with those.

Fixes #4837

@dcbaker dcbaker added the WIP label Mar 25, 2019
linker = AppleDynamicLinker(compiler, compiler_type)
else:
linker = GNUGoldDDynamicLinker(compiler, compiler_type)

Copy link
Member

Choose a reason for hiding this comment

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

[Flake8] [W293] blank line contains whitespace (link)

posted by Sider

def get_linker_search_args(self, dirname: str) -> List[str]:
return ['/LIBPATH:' + dirname]

def get_linker_output_args(self, outputname: str) -> List[str]:
Copy link
Member

Choose a reason for hiding this comment

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

[Flake8] [F811] redefinition of unused 'get_linker_output_args' from line 478 (link)

posted by Sider

There are two problems, one is that it assumes -flto is the argument
to do LTO/WPO, which isn't true of ICC and MSVC (and presumably)
others. It's also incorrect because it assumes that the compiler and
linker will always be the same, which isn't necessarily true. You
could combine GCC with Apple's linker, or clang with link.exe, which
use different arguments.
MSVC needs an argument to static linkers for doing LTO, so we need a way
for the static linker to tell us that.
This is the start of splitting the dynamic (not-static) linker out of
the compiler class. This just adds the classes in the linkers module,
but doesn't yet use them or alter any of the logic in the compilers
classes.
Ultimately this isn't what I want, I really want to have the linker as
a spearate object at the peer level to the compiler, however, to get
there I want the compiler object to wrap the linker object and
continue to expose the same interace.

Much of the reason to make them peers is that the linker often needs
to know about the compiler as it's invoked by the compiler.
Most compilers (apart from MSVC) are used to invoke the dynamic
linker, instead of invoking it directly. For these cases we combine
the linker and compiler args, as the compiler will read them and apply
the correct linker args for us (such as when using lto).

This property will be necessary once the dynamic linker and the
compiler representations are separated.
This is the stepping stone to completely splitting the compiler and
linker representation. This patch just uses the linker object to
fulfill the interfaces from the compiler object.

This is tested with run_unittests and is working on GCC, Clang, and
ICC on Linux. I still need to test on MacOS and Windows. I haven't
tested with alternative D compilers, I also haven't tested swift at
all.
def get_linker_search_args(self, dirname: str) -> List[str]:
return ['/LIBPATH:' + dirname]

def get_linker_output_args(self, outputname: str) -> List[str]:
Copy link
Member

Choose a reason for hiding this comment

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

[Flake8] [F811] redefinition of unused 'get_linker_output_args' from line 481 (link)

posted by Sider

linker = AppleDynamicLinker(compiler, compiler_type)
else:
linker = GNUGoldDDynamicLinker(compiler, compiler_type)

Copy link
Member

Choose a reason for hiding this comment

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

[Flake8] [W293] blank line contains whitespace (link)

posted by Sider

@jon-turney
Copy link
Member

For comparison jon-turney@8bac869 is as far as I got with this, which follows the same general idea of initially making the Compiler object just forward linker related method calls to a Linker object (but ends up looking quite different, somehow)

While I think this is the right set of changes for an initial pass of the idea, I'd like to make a couple more changes, but in separate series:

* Break the DynamicLinker out of the compiler as a peer object

* Add a generic way to change the linker, such as via an `LD` field in cross/native files

* Detect the linker in use and use the correct representation, like we do for compilers.

I agree with this approach :)

@dcbaker
Copy link
Member Author

dcbaker commented May 7, 2019

I'm closing in on getting the ICL stuff ready for review, and this is probably the next big thing for me after that. I'll need to rope in help from a few people as meson supports quite a few compiler/linker combinations now and I don't have access to many of them.

@dcbaker dcbaker closed this Jul 18, 2019
@Ericson2314
Copy link
Member

Successor #5681

@dcbaker dcbaker deleted the dynamic-linker-split-2 branch July 19, 2019 15:56
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.

Separate shared linker and compiler representations
4 participants