-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
Conversation
On MSVC, I was able to bootstrap, and build 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: |
/// 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`. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
@bors r+ |
📌 Commit cb23a79 has been approved by |
☀️ Test successful - checks-actions |
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