-
Notifications
You must be signed in to change notification settings - Fork 199
Update Generic Unix linker selection #1545
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
Conversation
swiftlang/llvm-project#8214 |
There was a problem hiding this 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.
@swift-ci please test Windows platform |
There was a problem hiding this 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.
5bd86f6
to
9920128
Compare
@swift-ci please test |
There was a problem hiding this 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.
@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
9920128
to
c11743b
Compare
@swift-ci please test |
The spelling of the linker is `-fuse-ld=`, not `--fuse-ld=`, as it can be seen in [1] and as used in the C++ driver in [2]. The code was correct before swiftlang#1545 using `-fuse-ld=`, but was changed to `--fuse-ld=`. The test was testing for the command line containing the same spelling, so it would had never failed, but running the actual command line would had failed. That code was eventually reverted and reverted back in swiftlang#1608. [1]: https://github.com/apple/llvm-project/blob/e33819dff1cbf7a90b0216a4993126cb11440d45/clang/include/clang/Driver/Options.td#L5267 [2]: https://github.com/apple/swift/blob/319f36b01baa082fd2d2640d6e320594752702d0/lib/Driver/UnixToolChains.cpp#L234
@MaxDesiatov @etcwilde This has been merged in Can we include this change in one of the next release ? |
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 swiftlang/swift#72061 merges as we won't know when to pass
nostart-stop-gc
with this change.Fixes: rdar://123061492