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

Update Generic Unix linker selection #1545

Merged
merged 2 commits into from May 6, 2024

Conversation

etcwilde
Copy link
Member

@etcwilde etcwilde commented Feb 16, 2024

Not all generic-"unix" environments have the Gold linker available to them, and in some cases, the vendor of the toolchain may provide their own linker. In these cases, the driver should be internally consistent with the toolchain that it is shipped with.

Now that we have the clang-linker, we can lean on the linker selection in the clang-linker to determine a default linker. If the clang-linker, and thus, the swift compiler driver, are part of a specific toolchain, that clang-linker should be built for that platform with the appropriate linker defaults set. If someone overrides the linker with -use-ld, we should still honour that, but should otherwise be consistent with the appropriate toolchain linker.

This will enable building and linking on AmazonLinux 2023 without requiring folks to always pass additional flags. It also means we can produce a self-contained toolchain that is internally consistent using lld as the linker.

LLD removes the start/stop symbols as of https://reviews.llvm.org/D96914, which the Swift runtime used to register the runtime type metadata and other runtime metadata tables. I'm waiting to merge this until apple/swift#72061 merges as we won't know when to pass nostart-stop-gc with this change.

Fixes: rdar://123061492

@etcwilde
Copy link
Member Author

apple/llvm-project#8214
@swift-ci please test

Copy link
Collaborator

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

This seems like the right approach, we use clang as the linker driver to avoid re-constructing the full linker invocation logic.

@compnerd
Copy link
Collaborator

@swift-ci please test Windows platform

Copy link
Contributor

@finagolfin finagolfin left a comment

Choose a reason for hiding this comment

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

LGTM, but test nits.

@etcwilde
Copy link
Member Author

etcwilde commented Mar 4, 2024

@swift-ci please test

Copy link
Contributor

@finagolfin finagolfin left a comment

Choose a reason for hiding this comment

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

Thanks, this is a good change that we probably should've been doing all along.

@MaxDesiatov
Copy link
Member

@swift-ci test windows

Not all generic-"unix" environments have the Gold linker available to
them, and in some cases, the vendor of the toolchain may provide their
own linker. In these cases, the driver should be internally consistent
with the toolchain that it is shipped with.

Now that we have the clang-linker, we can lean on the linker selection
in the clang-linker to determine a default linker. If the clang-linker,
and thus, the swift compiler driver, are part of a specific toolchain,
that clang-linker should be built for that platform with the appropriate
linker defaults set. If someone overrides the linker with
`-use-ld`, we should still honour that, but should otherwise be
consistent with the appropriate toolchain linker.

Fixes: rdar://123061492
@etcwilde
Copy link
Member Author

etcwilde commented May 4, 2024

@swift-ci please test

@etcwilde etcwilde merged commit 9bd28ec into apple:main May 6, 2024
3 checks passed
@etcwilde etcwilde deleted the ewilde/linker-selection branch May 6, 2024 20:03
@MaxDesiatov MaxDesiatov added the bug Something isn't working label May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants