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.

Sign up for GitHub

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

Jump to bottom

MVP for using rust-lld as part of cc #85961

Merged
merged 1 commit into from
Jun 11, 2021
Merged

MVP for using rust-lld as part of cc #85961

merged 1 commit into from
Jun 11, 2021

Conversation

sledgehammervampire
Copy link
Contributor

Will fix #71519. I need to figure out how to write a test showing that lld is used instead of whatever linker cc normally uses. When I manually run rustc using echo 'fn main() {}' | RUSTC_LOG=rustc_codegen_ssa::back::link=debug ./rustc -Clinker-flavor=gcc-lld --crate-type bin -Clink-arg=-Wl,-v (thanks to bjorn3 on Zulip), I can see that lld is used, but I'm not sure how to inspect that output in a test.

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 3, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@sledgehammervampire
Copy link
Contributor Author

sledgehammervampire commented Jun 3, 2021 •
edited
Loading

Hmm, it looks like the test passed on my machine because I had lld = true in my config.toml and used the rust-lld built by x.py.

@sledgehammervampire sledgehammervampire force-pushed the issue-71519-fix branch 3 times, most recently from 08bad9b to 2f5cde9 Compare June 4, 2021 14:22
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
bjorn3
bjorn3 reviewed Jun 6, 2021
View reviewed changes
src/bootstrap/compile.rs Outdated Show resolved Hide resolved
bjorn3
bjorn3 reviewed Jun 7, 2021
View reviewed changes
src/bootstrap/compile.rs Outdated Show resolved Hide resolved
bjorn3
bjorn3 reviewed Jun 7, 2021
View reviewed changes
src/bootstrap/compile.rs Outdated Show resolved Hide resolved
@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 10, 2021
@sledgehammervampire
gcc-lld mvp
2a76762 
ignore test if rust-lld not found

create ld -> rust-lld symlink at build time instead of run time

for testing in ci

copy instead of symlinking

remove linux check

test for linker, suggestions from bjorn3

fix overly restrictive lld matcher

use -Zgcc-ld flag instead of -Clinker-flavor

refactor code adding lld to gcc path

revert ci changes

suggestions from petrochenkov

rename gcc_ld to gcc-ld in dirs
@sledgehammervampire
Copy link
Contributor Author

I've fixed the conflict (it was just merging the import sets from the new master and my PR).

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 10, 2021

📌 Commit 2a76762 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 10, 2021
@bors
Copy link
Contributor

bors commented Jun 11, 2021

⌛ Testing commit 2a76762 with merge 72868e0...

@bors
Copy link
Contributor

bors commented Jun 11, 2021

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 72868e0 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 11, 2021
@bors bors merged commit 72868e0 into rust-lang:master Jun 11, 2021
@rustbot rustbot added this to the 1.54.0 milestone Jun 11, 2021
@bors bors mentioned this pull request Jun 11, 2021
Merged
@sledgehammervampire
Copy link
Contributor Author

sledgehammervampire commented Jun 12, 2021 •
edited
Loading

@petrochenkov I was trying to dogfood my own PR, and I realized that I copied "rust-lld" as "rust-lld" instead of "ld" in gcc_ld_dir in src/bootstrap/dist.rs. Should I make a new PR?

Edit: I renamed "rust-lld" to "ld" in my local copy of the nightly toolchain and I get errors that look like

linking with `cc` failed: exit status: 1
<long cc invocation>
collect2: fatal error: execvp: No such file or directory

@petrochenkov
Copy link
Contributor

@1000teslas
Yes, pleas make a new PR.

I get errors that look like

Looks like the file passes the exists check, but then cannot actually be executed.
Could you check whether it has executable rights?

@sledgehammervampire
Copy link
Contributor Author

@petrochenkov

Looks like the file passes the exists check, but then cannot actually be executed. Could you check whether it has executable rights?

Are you talking about the copy of rust-lld as ld? If so, then yes.

sledgehammervampire referenced this pull request in sledgehammervampire/rust Jun 30, 2021
@sledgehammervampire
copy rust-lld as ld in dist
fe822fe
@sledgehammervampire sledgehammervampire mentioned this pull request Jun 30, 2021
Merged
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 30, 2021
@JohnTitor
Rollup merge of rust-lang#86740 - 1000teslas:issue-71519-fix, r=petro…
e52a0ec 
…chenkov

copy rust-lld as ld in dist

Fixes bug in rust-lang#85961. Linking seems to work for pure Rust projects, but not when a C library needs to be dynamically linked.
@oxalica oxalica mentioned this pull request Jul 28, 2021
Closed
@hkratz hkratz mentioned this pull request Aug 23, 2021
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 26, 2021
@Dylan-DPC
Rollup merge of rust-lang#88250 - rusticstuff:macos-lld, r=nagisa
5fb3ca9 
Make `-Z gcc-ld=lld` work for Apple targets

`-Z gcc-ld=lld` was introduced in rust-lang#85961. It does not work on Macos because lld needs be either named `ld64` or passed `-flavor darwin` as the first two arguments in order to select the Mach-O flavor. Rust invokes cc (=clang) on Macos for linking which calls `ld` as linker binary and not `ld64`, so just creating an `ld64` binary and modifying the search path with `-B` does not work.

In order to solve this patch does:
* Set the `lld_flavor` for all Apple-derived targets to `LldFlavor::Ld64`. As far as I can see this actually works towards fixing `-Xlinker=rust-lld` as all those targets use the Mach-O object format.
* Copy/hardlink rust-lld to the gcc-ld subdirectory as ld64 next to ld.
* If `-Z gcc-ld=lld` is used and the target lld flavor is Ld64 add `-fuse-ld=/path/to/ld64` to the linker invocation.

Fixes rust-lang#86945.
Manishearth added a commit to Manishearth/rust that referenced this pull request Aug 27, 2021
@Manishearth
Rollup merge of rust-lang#88250 - rusticstuff:macos-lld, r=nagisa
8bd0fe5 
Make `-Z gcc-ld=lld` work for Apple targets

`-Z gcc-ld=lld` was introduced in rust-lang#85961. It does not work on Macos because lld needs be either named `ld64` or passed `-flavor darwin` as the first two arguments in order to select the Mach-O flavor. Rust invokes cc (=clang) on Macos for linking which calls `ld` as linker binary and not `ld64`, so just creating an `ld64` binary and modifying the search path with `-B` does not work.

In order to solve this patch does:
* Set the `lld_flavor` for all Apple-derived targets to `LldFlavor::Ld64`. As far as I can see this actually works towards fixing `-Xlinker=rust-lld` as all those targets use the Mach-O object format.
* Copy/hardlink rust-lld to the gcc-ld subdirectory as ld64 next to ld.
* If `-Z gcc-ld=lld` is used and the target lld flavor is Ld64 add `-fuse-ld=/path/to/ld64` to the linker invocation.

Fixes rust-lang#86945.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 27, 2021
@Dylan-DPC
Rollup merge of rust-lang#88250 - rusticstuff:macos-lld, r=nagisa
93415ff 
Make `-Z gcc-ld=lld` work for Apple targets

`-Z gcc-ld=lld` was introduced in rust-lang#85961. It does not work on Macos because lld needs be either named `ld64` or passed `-flavor darwin` as the first two arguments in order to select the Mach-O flavor. Rust invokes cc (=clang) on Macos for linking which calls `ld` as linker binary and not `ld64`, so just creating an `ld64` binary and modifying the search path with `-B` does not work.

In order to solve this patch does:
* Set the `lld_flavor` for all Apple-derived targets to `LldFlavor::Ld64`. As far as I can see this actually works towards fixing `-Xlinker=rust-lld` as all those targets use the Mach-O object format.
* Copy/hardlink rust-lld to the gcc-ld subdirectory as ld64 next to ld.
* If `-Z gcc-ld=lld` is used and the target lld flavor is Ld64 add `-fuse-ld=/path/to/ld64` to the linker invocation.

Fixes rust-lang#86945.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 27, 2021
@Dylan-DPC
Rollup merge of rust-lang#88250 - rusticstuff:macos-lld, r=nagisa
056809f 
Make `-Z gcc-ld=lld` work for Apple targets

`-Z gcc-ld=lld` was introduced in rust-lang#85961. It does not work on Macos because lld needs be either named `ld64` or passed `-flavor darwin` as the first two arguments in order to select the Mach-O flavor. Rust invokes cc (=clang) on Macos for linking which calls `ld` as linker binary and not `ld64`, so just creating an `ld64` binary and modifying the search path with `-B` does not work.

In order to solve this patch does:
* Set the `lld_flavor` for all Apple-derived targets to `LldFlavor::Ld64`. As far as I can see this actually works towards fixing `-Xlinker=rust-lld` as all those targets use the Mach-O object format.
* Copy/hardlink rust-lld to the gcc-ld subdirectory as ld64 next to ld.
* If `-Z gcc-ld=lld` is used and the target lld flavor is Ld64 add `-fuse-ld=/path/to/ld64` to the linker invocation.

Fixes rust-lang#86945.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2021
@bors
Auto merge of rust-lang#88250 - rusticstuff:macos-lld, r=nagisa
757a65b 
Make `-Z gcc-ld=lld` work for Apple targets

`-Z gcc-ld=lld` was introduced in rust-lang#85961. It does not work on Macos because lld needs be either named `ld64` or passed `-flavor darwin` as the first two arguments in order to select the Mach-O flavor. Rust invokes cc (=clang) on Macos for linking which calls `ld` as linker binary and not `ld64`, so just creating an `ld64` binary and modifying the search path with `-B` does not work.

In order to solve this patch does:
* Set the `lld_flavor` for all Apple-derived targets to `LldFlavor::Ld64`. As far as I can see this actually works towards fixing `-Xlinker=rust-lld` as all those targets use the Mach-O object format.
* Copy/hardlink rust-lld to the gcc-ld subdirectory as ld64 next to ld.
* If `-Z gcc-ld=lld` is used and the target lld flavor is Ld64 add `-fuse-ld=/path/to/ld64` to the linker invocation.

Fixes rust-lang#86945.
@ghost ghost mentioned this pull request Sep 12, 2021
Closed
@lqd lqd mentioned this pull request Apr 14, 2022
Closed
3 tasks
@lqd lqd mentioned this pull request May 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

@bjorn3 bjorn3 Awaiting requested review from bjorn3

Assignees

@petrochenkov petrochenkov

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.

Support feeding rust-lld into gcc (and clang)
8 participants
@sledgehammervampire @rust-highfive @rust-log-analyzer @bjorn3 @petrochenkov @bors @davidtwco @rustbot