-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Debugflag: -Z emit-end-regions #44249
Conversation
r? @arielb1 (rust_highfive has picked a reviewer for you, use r? to override) |
I switched to the approach of turning off Why is this is right path: the |
Could we have Also maybe the two flags, being similar in nature, should have similar names. E.g., the validation flag could be renamed to |
☔ The latest upstream changes (presumably #44253) made this pull request unmergeable. Please resolve the merge conflicts. |
@RalfJung I figured changes like that could wait for a follow-up PR; I want this one to be somewhat minimal so that it is easier to back port. Do you have any objection to that plan? |
Merging this as-is will make miri's test suite fail as validation will entirely break. So if we have one flag imply the other, I'd prefer if we did that immediately rather than patching miri twice.
Am 4. September 2017 11:08:52 MESZ schrieb Felix S Klock II <[email protected]>:
…
@RalfJung I figured changes like that could wait for a follow-up PR; I
want this one to be somewhat minimal so that it is easier to back port.
Do you have any objection to that plan?
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#44249 (comment)
|
…able it. The main intent is to fix cases where EndRegion emission is believed to be causing excess peak memory pressure. It may also be a welcome change to people inspecting the MIR output who find the EndRegions to be a distraction.
Okay I'll see about making |
…N > 0). This way the miri test suite does not have to be updated to explcitly request `-Z emit-end-regions`.
7875220
to
f2892ad
Compare
Looks good, thanks! |
@bors r+ |
📌 Commit f2892ad has been approved by |
Debugflag: -Z emit-end-regions Skip EndRegion emission by default. Use `-Z emit-end-regions` to reenable it. The main intent is to fix cases where `EndRegion` emission is believed to be causing excess peak memory pressure. It may also be a welcome change to people inspecting the MIR output who find the EndRegions to be a distraction. (In later follow-up PR's I will put in safe-guards against using the current mir-borrowck without enabling `EndRegion` emission. But I wanted this PR to be minimal, in part because we may wish to backport it to the beta channel if we find that it reduces peak memory usage significantly.)
☀️ Test successful - status-appveyor, status-travis |
Skip EndRegion emission by default. Use
-Z emit-end-regions
to reenable it.The main intent is to fix cases where
EndRegion
emission is believed to be causing excess peak memory pressure.It may also be a welcome change to people inspecting the MIR output who find the EndRegions to be a distraction.
(In later follow-up PR's I will put in safe-guards against using the current mir-borrowck without enabling
EndRegion
emission. But I wanted this PR to be minimal, in part because we may wish to backport it to the beta channel if we find that it reduces peak memory usage significantly.)