Skip to content

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Apr 29, 2021

Since we're supported "MSVC-style" EH codegen (via cleanup_pad and funclets), we've had the analysis needed to be able to also construct "GNU-style" landing_pad blocks ahead of time (like we do for cleanup_pads).

The main reason not to do it eagerly is if they can get skipped often enough to reduce compile times.
(I'm hoping a perf run will be enough to reveal whether that is the case - at least this time it's not MSVC)

r? @nagisa

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 29, 2021
@eddyb
Copy link
Member Author

eddyb commented Apr 29, 2021

@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 Apr 29, 2021
@bors
Copy link
Collaborator

bors commented Apr 29, 2021

⌛ Trying commit ac3a8f9 with merge 7e27bad92f74591bb162eaff71e2f9d70a698c5f...

@bors
Copy link
Collaborator

bors commented Apr 29, 2021

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

@rust-timer
Copy link
Collaborator

Queued 7e27bad92f74591bb162eaff71e2f9d70a698c5f with parent 10a51c0, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (7e27bad92f74591bb162eaff71e2f9d70a698c5f): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 29, 2021
@eddyb
Copy link
Member Author

eddyb commented Apr 29, 2021

Not very clear-cut, but it does look like there are some effects.
I might have to rethink this whole thing and "just" figure out how to generate the same IR as today, still on-the-fly, but with a different approach to Builder ownership.

@eddyb
Copy link
Member Author

eddyb commented May 1, 2021

I may abandon this in favor of pursuing something based on #84771, instead - I'm really happy how that turned out.

@eddyb
Copy link
Member Author

eddyb commented May 15, 2021

Closing as fully obsoleted by #85316.

@eddyb eddyb closed this May 15, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request May 16, 2021
…nagisa

rustc_codegen_ssa: generate MSVC cleanup pads on demand, like GNU landing pads.

This unblocks rust-lang#84993 in terms of codegen tests, as it brings the MSVC-style (`cleanup_pad`) EH (LLVM) block order in line with the GNU-style (`landing_pad`) EH (LLVM) block order, by having both of them be on-demand (instead of MSVC-style being eager and GNU-style lazy/on-demand).

It also unifies the two implementations a bit, similar to rust-lang#84699, but in the opposite direction (as that attempt made both kinds of EH pads eagerly built).

~~Opening as draft because I haven't done enough Windows testing just yet, of both this PR, and of rust-lang#84993 rebased on it.~~ (**EDIT**: seems to be working as expected)

r? `@nagisa`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants