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

change stdlib circular dependencies handling #100404

Merged
merged 2 commits into from
Sep 7, 2022
Merged

change stdlib circular dependencies handling #100404

merged 2 commits into from
Sep 7, 2022

Conversation

belovdv
Copy link
Contributor

@belovdv belovdv commented Aug 11, 2022 •
edited by petrochenkov
Loading

Remove group_start and group_end, add dependencies to symbols.o
Implements the suggestion from #85805 (comment)
r? @petrochenkov

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 11, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 11, 2022
@belovdv
Copy link
Contributor Author

belovdv commented Aug 11, 2022

@rustbot author

@rustbot rustbot 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 Aug 11, 2022
@belovdv belovdv closed this Aug 15, 2022
@belovdv belovdv reopened this Aug 15, 2022
@belovdv
Copy link
Contributor Author

belovdv commented Aug 15, 2022

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 15, 2022
@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 Aug 15, 2022
@petrochenkov petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 16, 2022
@petrochenkov
Copy link
Contributor

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 16, 2022
@bors
Copy link
Contributor

bors commented Aug 16, 2022

⌛ Trying commit 819daad38be437c3bb16a8b80bb01fb82cd367cc with merge b069a434200d6e5f6a8497bd62344966d1c24d3c...

@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 Sep 6, 2022
@bors
Copy link
Contributor

bors commented Sep 6, 2022

⌛ Testing commit c34047c with merge aed0a7b6c7bb973505567f10de217731597d1537...

@bors
Copy link
Contributor

bors commented Sep 6, 2022

💔 Test failed - checks-actions

@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 Sep 6, 2022
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@petrochenkov
Copy link
Contributor

No output from the builder, weird.
@bors retry

@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 Sep 6, 2022
@bors
Copy link
Contributor

bors commented Sep 6, 2022

⌛ Testing commit c34047c with merge 699bfa8...

@bors
Copy link
Contributor

bors commented Sep 7, 2022

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 699bfa8 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 7, 2022
@bors bors merged commit 699bfa8 into rust-lang:master Sep 7, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 7, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (699bfa8): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)		 1.2% [1.1%, 1.3%] 2
Regressions ❌
(secondary)		 3.4% [3.0%, 3.9%] 6
Improvements ✅
(primary)		 -0.8% [-0.9%, -0.7%] 6
Improvements ✅
(secondary)		 -1.0% [-1.0%, -1.0%] 1
All ❌✅ (primary) -0.3% [-0.9%, 1.3%] 8

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 4.8% [4.8%, 4.8%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -3.4% [-3.4%, -3.4%] 1
All ❌✅ (primary) - - 0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)		 2.8% [2.8%, 2.8%] 1
Regressions ❌
(secondary)		 2.1% [2.1%, 2.1%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 2.8% [2.8%, 2.8%] 1

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2 ↩3

  2. number of relevant changes ↩ ↩2 ↩3

@rustbot rustbot added the perf-regression Performance regression. label Sep 7, 2022
@nnethercote
Copy link
Contributor

The html5ever and keccak benchmarks have been bimodal and thus misleading lately, so those regressions probably aren't real. The cranelift-codegen ones I'm not sure about, they may or may not be real.

@pnkfelix
Copy link
Member

I think subsequent evidence from past week shows that cranelift-codegen is also behaving in a bimodal manner.

@rustbot label: perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Sep 14, 2022
@apiraino apiraino mentioned this pull request Sep 26, 2022
Closed
@messense messense mentioned this pull request Sep 30, 2022
Closed
@petrochenkov petrochenkov mentioned this pull request Oct 15, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 20, 2022
@bors
Auto merge of rust-lang#103092 - petrochenkov:weaklto, r=wesleywiser
5ffa67d 
linker: Fix weak lang item linking with combination windows-gnu + LLD + LTO

In rust-lang#100404 this logic was originally disabled for MSVC due to issues with LTO, but the same issues appear on windows-gnu with LLD because that LLD uses the same underlying logic as MSVC LLD, just with re-syntaxed command line options.

So this PR just disables it for LTO builds in general.
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Oct 21, 2022
@bors
Auto merge of #103092 - petrochenkov:weaklto, r=wesleywiser
d792546 
linker: Fix weak lang item linking with combination windows-gnu + LLD + LTO

In rust-lang/rust#100404 this logic was originally disabled for MSVC due to issues with LTO, but the same issues appear on windows-gnu with LLD because that LLD uses the same underlying logic as MSVC LLD, just with re-syntaxed command line options.

So this PR just disables it for LTO builds in general.
@petrochenkov petrochenkov mentioned this pull request Oct 23, 2022
Merged
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 20, 2024
@bors
Auto merge of #103092 - petrochenkov:weaklto, r=wesleywiser
5f13dcd 
linker: Fix weak lang item linking with combination windows-gnu + LLD + LTO

In rust-lang/rust#100404 this logic was originally disabled for MSVC due to issues with LTO, but the same issues appear on windows-gnu with LLD because that LLD uses the same underlying logic as MSVC LLD, just with re-syntaxed command line options.

So this PR just disables it for LTO builds in general.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
@bors
Auto merge of #103092 - petrochenkov:weaklto, r=wesleywiser
a17fa9a 
linker: Fix weak lang item linking with combination windows-gnu + LLD + LTO

In rust-lang/rust#100404 this logic was originally disabled for MSVC due to issues with LTO, but the same issues appear on windows-gnu with LLD because that LLD uses the same underlying logic as MSVC LLD, just with re-syntaxed command line options.

So this PR just disables it for LTO builds in general.
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

@nbdd0121 nbdd0121 nbdd0121 left review comments

@bjorn3 bjorn3 bjorn3 approved these changes

Assignees

@petrochenkov petrochenkov

Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.65.0
Development

Successfully merging this pull request may close these issues.