-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
-Zvalidate-mir: Assert that storage is allocated on local use #77369
-Zvalidate-mir: Assert that storage is allocated on local use #77369
Conversation
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
r? @wesleywiser We now run an extra data flow pass, so this could impact perf. @bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit 36365b35ee72d8cfde7949f55517832878a754c0 with merge d952cf6887b310e5009157bddd5894b5f54cb31b... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors r- |
@bors retry |
@bors try |
⌛ Trying commit 36365b35ee72d8cfde7949f55517832878a754c0 with merge 1bdd5353b898fca7dc1cc8e53331d72c296eea8a... |
This needs #77396 before bootstrapping will work |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-actions |
Pushed a temp. commit that disables the pass to get perf data @bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit e32bb31b958b87a677515ca5881169f88a20aca6 with merge 6c70b42c961226e0059a4aa0bef7f315aad8ad8c... |
☀️ Try build successful - checks-actions, checks-azure |
Queued 6c70b42c961226e0059a4aa0bef7f315aad8ad8c with parent 8fe73e8, future comparison URL. |
Finished benchmarking try commit (6c70b42c961226e0059a4aa0bef7f315aad8ad8c): 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 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 |
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.
This looks great and #77396 just merged so you should be able to bootstrap now.
@bors r=wesleywiser |
📌 Commit c47011f has been approved by |
⌛ Testing commit c47011f with merge c7ec262b251125405b06f5b4f71a143146f54a93... |
💔 Test failed - checks-actions |
@bors retry |
☀️ Test successful - checks-actions, checks-azure |
…cation, r=wesleywiser Disable MatchBranchSimplification This optimization can result in unsoundness, because it introduces additional uses of a place holding the discriminant value without ensuring that it is valid to do so. Found by validation from rust-lang#77369 / rust-lang#78147.
This extends the MIR validator to check that locals are only used when their backing storage is currently allocated via
StorageLive
.The result of this is that miscompilations such as #77359 are caught and turned into ICEs.
The PR currently fails tests because miscompilations such as #77359 are caught and turned into ICEs.
I have confirmed that tests pass (even with
-Zvalidate-mir
) onceSimplifyArmIdentity
is turned into a no-op (except mir-opt tests, of course).