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

linker: Refactoring and fixes to native library linking #103311

Merged
merged 6 commits into from
Nov 13, 2022
Merged

linker: Refactoring and fixes to native library linking #103311

merged 6 commits into from
Nov 13, 2022

Conversation

petrochenkov
Copy link
Contributor

This PR contains a bunch of code cleanup and comment rearrangements + 2 fixes for -Zpacked-bundled-libs.
It's better to look at individual commits.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 20, 2022
@rust-highfive
Copy link
Collaborator

r? @nagisa

(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 Oct 20, 2022
@petrochenkov
Copy link
Contributor Author

cc @belovdv

@petrochenkov
Copy link
Contributor Author

The failing run-make-fulldeps test needs to be updated, adjacent identical native libraries are no longer deduplicated if they come from different crates, but that's fine, it's not the main motivation.

@bors

This comment was marked as resolved.

Sign in to view
@nagisa
Copy link
Member

nagisa commented Nov 6, 2022

r? @bjorn3 seems to be way more on-top of this than I am.

@rustbot rustbot assigned bjorn3 and unassigned nagisa Nov 6, 2022
@petrochenkov
Copy link
Contributor Author

Updated with the change from #103311 (comment) removed.
@rustbot ready

@bjorn3
Copy link
Member

bjorn3 commented Nov 12, 2022

@bors r+ rollup=iffy

@bors
Copy link
Contributor

bors commented Nov 12, 2022

📌 Commit edb9175eb4e0641138f9d90d4877f86469b3a950 has been approved by bjorn3

It is now in the queue for this repository.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 12, 2022
@bors
Copy link
Contributor

bors commented Nov 12, 2022

⌛ Testing commit edb9175eb4e0641138f9d90d4877f86469b3a950 with merge e1392304fca9da9c1e1a81251f0384456380812c...

@bors

This comment was marked as resolved.

Sign in to view
@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 12, 2022
@rust-log-analyzer

This comment was marked as resolved.

Sign in to view
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 12, 2022
@petrochenkov
linker: Factor out native library linking to a separate function
cae3c93
@petrochenkov
linker: Simplify linking of compiler_builtins and profiler_builtins
e792de2 
This also fixes linking of native libraries bundled into these crates when `-Zpacked-bundled-libs` is enabled
@petrochenkov
linker: Move some inner functions to the outside
fe7aab1 
Inline `fn unlib`
@petrochenkov
linker: Support mixing crates built with different values of `-Zpacke…
82ecfd4 
…d_bundled_libs`

So you can change the value of `-Zpacked_bundled_libs` without rebuilding standard library
@petrochenkov
Update run-make-fulldeps tests
58e4644 
Adjacent identical native libraries are no longer deduplicated if they come from different crates
@petrochenkov
linker: Link profiler_builtins even if it's marked as NotLinked
c2358a1
@petrochenkov
Copy link
Contributor Author

I've added a commit to link profiler_builtins even if is's marked as Linkage::NotLinked.
It shouldn't be NotLinked, the fact that it's marked as such is some pre-existing issue in dependency_format.rs, so I've also added a FIXME comment.
@bors r=bjorn3

@bors
Copy link
Contributor

bors commented Nov 12, 2022

📌 Commit c2358a1 has been approved by bjorn3

It is now in the queue for this repository.

@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 Nov 12, 2022
@bjorn3
Copy link
Member

bjorn3 commented Nov 12, 2022

It's not the nicest solution, but it should work for now.

@bors r+

@bors
Copy link
Contributor

bors commented Nov 12, 2022

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Nov 12, 2022

📌 Commit c2358a1 has been approved by bjorn3

It is now in the queue for this repository.

@bjorn3
Copy link
Member

bjorn3 commented Nov 12, 2022

Didn't see you already posted an r+ :)

@bors
Copy link
Contributor

bors commented Nov 12, 2022

⌛ Testing commit c2358a1 with merge fb6667a...

@bors
Copy link
Contributor

bors commented Nov 13, 2022

☀️ Test successful - checks-actions
Approved by: bjorn3
Pushing fb6667a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 13, 2022
@bors bors merged commit fb6667a into rust-lang:master Nov 13, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 13, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (fb6667a): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

@petrochenkov petrochenkov mentioned this pull request Nov 14, 2022
Closed
@mkroening mkroening mentioned this pull request Dec 12, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjorn3 bjorn3 bjorn3 left review comments

@klensy klensy klensy left review comments

Assignees

@bjorn3 bjorn3

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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.67.0
Development

Successfully merging this pull request may close these issues.
9 participants
@petrochenkov @rust-highfive @bors @nagisa @bjorn3 @rust-log-analyzer @rust-timer @klensy @rustbot