Skip to content

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

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

Merged
merged 1 commit into from
May 16, 2021

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented May 15, 2021

This unblocks #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 #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 #84993 rebased on it. (EDIT: seems to be working as expected)

r? @nagisa

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

eddyb commented May 15, 2021

On MSVC, I was able to bootstrap, and build cargo, ripgrep and syn, with this PR.

Also, rebasing #84993 on this PR did indeed fix the test failure there (i.e. MSVC and GNU styles indeed agree on the LLVM block order).

So I'm un-drafting this PR, it's ready for review I think.

Also, in case anything goes wrong, in the interest of easy bisecting:
@bors rollup=never

@eddyb eddyb marked this pull request as ready for review May 15, 2021 08:29
/// This stores the landing-pad block for a given BB, computed lazily on GNU
/// and eagerly on MSVC.
/// This stores the cached landing/cleanup pad block for a given BB.
// FIXME(eddyb) rename this to `eh_pads`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason these renames aren't happening now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add a commit for them if you want, no particular reason, was focusing on getting it working and tested first.

@nagisa
Copy link
Member

nagisa commented May 16, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented May 16, 2021

📌 Commit cb23a79 has been approved by nagisa

@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 May 16, 2021
@bors
Copy link
Collaborator

bors commented May 16, 2021

⌛ Testing commit cb23a79 with merge 747a5d2...

@bors
Copy link
Collaborator

bors commented May 16, 2021

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 747a5d2 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 16, 2021
@bors bors merged commit 747a5d2 into rust-lang:master May 16, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 16, 2021
@eddyb eddyb deleted the cg-ssa-on-demand-cleanuppad branch May 16, 2021 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

5 participants