Skip to content
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

Mir: remove Panic terminator and instead just call suitable lang item #29573

Closed
nikomatsakis opened this Issue Nov 4, 2015 · 8 comments

Comments

Projects
None yet
3 participants
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Nov 4, 2015

Panic in MIR is unimplemented. It should call into the panic lang item.

@wesleywiser

This comment has been minimized.

Copy link
Member

wesleywiser commented Nov 6, 2015

Hi, I'd like to give this a shot.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Nov 6, 2015

@wesleywiser great! If you want to chat about it, the easiest way is to privmsg me on IRC (nmatsakis) or send me an email (nmatsakis@mozilla.com). I will note though that it may be that we should reshape panics to just be calls, in which case this task is more about handling calls than about panics per se, which is maybe a bigger job (but a very important one).

@nikomatsakis nikomatsakis changed the title Mir: Trans Panics Mir: remove Panic terminator and instead just call suitable lang item Nov 11, 2015

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Nov 11, 2015

I now think we should refactor panics to simply call into the appropriate lang item and not have an explicit Panic terminator.

@nagisa

This comment has been minimized.

Copy link
Contributor

nagisa commented Nov 13, 2015

Replacing Panic terminator with explicit call to language item that’s followed by a Diverge (I assume Diverge has the same semantics as intrinsics::unreachable) terminator sounds good to me.

@nagisa

This comment has been minimized.

Copy link
Contributor

nagisa commented Nov 13, 2015

On the other hand, it would make implementing certain codegen features such as -C no-landing-pads near-impossible? Then probably doesn’t sound as good as it did before 😃

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Nov 13, 2015

@nagisa I don't see why it makes that any harder. In such a mode we presumably would not emit a Panic either, and/or we'd change the lang item to abort (and strip away the landing pads path so that the call block has no successors).

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Nov 13, 2015

@nagisa also, the semantics of diverge are not unreachable. The diverge
terminator continues unwinding. Perhaps it should have been called "resume".

On Fri, Nov 13, 2015 at 3:00 PM, Simonas Kazlauskas <
notifications@github.com> wrote:

On the other hand, it would make implementing certain codegen features
such as -C no-landing-pads near-impossible? Then probably doesn’t sounds
as good as it did before 😃


Reply to this email directly or view it on GitHub
#29573 (comment).

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Nov 13, 2015

Though, we could say that if landing pads are disabled, then the Diverge
terminator just traps, and then we can keep directing all panic pathways
directly into the Diverge block. Not sure, plenty of ok options I guess.

On Fri, Nov 13, 2015 at 3:07 PM, Nicholas Matsakis nmatsakis@mozilla.com
wrote:

@nagisa also, the semantics of diverge are not unreachable. The diverge
terminator continues unwinding. Perhaps it should have been called "resume".

On Fri, Nov 13, 2015 at 3:00 PM, Simonas Kazlauskas <
notifications@github.com> wrote:

On the other hand, it would make implementing certain codegen features
such as -C no-landing-pads near-impossible? Then probably doesn’t sounds
as good as it did before 😃


Reply to this email directly or view it on GitHub
#29573 (comment).

bors added a commit that referenced this issue Jan 6, 2016

Auto merge of #30481 - nagisa:mir-calls-2, r=nikomatsakis
r? @nikomatsakis

This is a pretty big PR conflating changes to a few different block terminators (Call, DivergingCall, Panic, Resume, Diverge), because they are somewhat closely related.

Each commit has a pretty good description on what is being changed in each commit. The end result is greatly simplified CFG and translation for calls (no success branch if the function is diverging, no cleanup branch if there’s nothing to cleanup etc).

Fixes #30480
Related #29767
Partialy solves #29575
Fixes #29573

bors added a commit that referenced this issue Jan 6, 2016

Auto merge of #30481 - nagisa:mir-calls-2, r=nikomatsakis
r? @nikomatsakis

This is a pretty big PR conflating changes to a few different block terminators (Call, DivergingCall, Panic, Resume, Diverge), because they are somewhat closely related.

Each commit has a pretty good description on what is being changed in each commit. The end result is greatly simplified CFG and translation for calls (no success branch if the function is diverging, no cleanup branch if there’s nothing to cleanup etc).

Fixes #30480
Fixes #29767
Partialy solves #29575
Fixes #29573

@bors bors closed this in #30481 Jan 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.