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

Reapply: Mark drop calls in landing pads cold instead of noinline #94823

Closed
wants to merge 2 commits into from

Conversation

erikdesjardins
Copy link
Contributor

@erikdesjardins erikdesjardins commented Mar 10, 2022

Reapplies #92419, which was reverted in #94402 due to #94390.

Fixes #46515, fixes #87055.

r? @ghost

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 10, 2022
@nikic
Copy link
Contributor

nikic commented Mar 11, 2022

@bors try

@bors
Copy link
Contributor

bors commented Mar 11, 2022

⌛ Trying commit 5266aa7 with merge ccd5064a169a0c1748c8555e905f9055884e0507...

@bors
Copy link
Contributor

bors commented Mar 11, 2022

☀️ Try build successful - checks-actions
Build commit: ccd5064a169a0c1748c8555e905f9055884e0507 (ccd5064a169a0c1748c8555e905f9055884e0507)

@Dylan-DPC
Copy link
Member

@erikdesjardins any updates on this?

@erikdesjardins
Copy link
Contributor Author

While I concur with nikic's analysis here (#94390 (comment)), I still doubt this will be accepted without further LLVM changes.

I guess draft PRs aren't excluded from triage?
@rustbot label S-blocked

@rustbot rustbot added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Apr 24, 2022
@ldm0
Copy link
Contributor

ldm0 commented Jul 31, 2022

@erikdesjardins Hi.

Noticable size bloat(about 2%) and codegen degradation happened on our private project after upgrading rustc. After bisection, I found that the problem was introduced in #94402, which could be resolved by this PR.

I tested locally and found that this PR will fix not only #46515, #87055, but also #97217 and #98679. As the compile time issue was trimmed(#94390 (comment)), could you push this PR forward?

@erikdesjardins
Copy link
Contributor Author

I would also like to see this relanded, but I still agree with my previous comment, so I'm not going to spend the time to update this PR. You have my blessing to rebase these commits and open your own PR if you want.

@InnovativeInventor
Copy link
Contributor

I would also like to see this relanded, but I still agree with my previous comment, so I'm not going to spend the time to update this PR. You have my blessing to rebase these commits and open your own PR if you want.

@erikdesjardins @ldm0 Since there didn't seem to be any activity on this, I took the liberty of rebasing this off master and opening a rebased PR. Let me know how I can help to move this along.

@Dylan-DPC
Copy link
Member

Closing this in favour of #102099

@Dylan-DPC Dylan-DPC closed this May 12, 2023
@Dylan-DPC Dylan-DPC removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label May 12, 2023
@erikdesjardins erikdesjardins deleted the re-cold-land branch May 12, 2023 21:28
TaKO8Ki added a commit to TaKO8Ki/rust that referenced this pull request Jun 27, 2023
…=nikic

Rebased: Mark drop calls in landing pads cold instead of noinline

I noticed that certain inlining optimizations were missing while staring at some compiled code output. I'd like to see this relanded, so I rebased the PR from ``@erikdesjardins`` (PR rust-lang#94823).

This PR reapplies rust-lang#92419, which was reverted in rust-lang#94402 due to rust-lang#94390.

Fixes rust-lang#46515, fixes rust-lang#87055.

Update: fixes rust-lang#97217.
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 2, 2023
…ikic

Rebased: Mark drop calls in landing pads cold instead of noinline

I noticed that certain inlining optimizations were missing while staring at some compiled code output. I'd like to see this relanded, so I rebased the PR from `@erikdesjardins` (PR rust-lang#94823).

This PR reapplies rust-lang#92419, which was reverted in rust-lang#94402 due to rust-lang#94390.

Fixes rust-lang#46515, fixes rust-lang#87055.

Update: fixes rust-lang#97217.
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 2, 2023
…ikic

Rebased: Mark drop calls in landing pads cold instead of noinline

I noticed that certain inlining optimizations were missing while staring at some compiled code output. I'd like to see this relanded, so I rebased the PR from `@erikdesjardins` (PR rust-lang#94823).

This PR reapplies rust-lang#92419, which was reverted in rust-lang#94402 due to rust-lang#94390.

Fixes rust-lang#46515, fixes rust-lang#87055.

Update: fixes rust-lang#97217.
antoyo pushed a commit to antoyo/rust that referenced this pull request Oct 9, 2023
…ikic

Rebased: Mark drop calls in landing pads cold instead of noinline

I noticed that certain inlining optimizations were missing while staring at some compiled code output. I'd like to see this relanded, so I rebased the PR from `@erikdesjardins` (PR rust-lang#94823).

This PR reapplies rust-lang#92419, which was reverted in rust-lang#94402 due to rust-lang#94390.

Fixes rust-lang#46515, fixes rust-lang#87055.

Update: fixes rust-lang#97217.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
7 participants