Stream: t-compiler/wg-mir-opt

Topic: unreachable canoncalization


rkruppe (Nov 14 2019 at 14:19, on Zulip):

So there's now two proposed MIR optimizations (#66282, #66329) that try to exploit "executing unreachable is UB" and need to take care that the Unreachable they're looking at is actually guaranteed to execute. I could imagine there's more in the future. Naively, this could require scanning lots of basic blocks.

It seems like, perhaps, there should be a clean-up which runs once in a while and which other passes rely on for quality of optimizations (not for correctness ofc) rather than redoing analyses. Similar to CFG simplification, if you will. In fact, maybe #66329 could be that pass.

Alternatively or in addition, as I sketched in https://github.com/rust-lang/rust/pull/66282#discussion_r346319446, we could trim s1; s2; ...; sN; unreachable to s1; s2; ...; sK; unreachable where sK is the last statement (if any) that can diverge (and thus, make the basic block not-UB). This could be very cheap in the case where nothing changes (only need to look at the last statement in the BB) so we could perhaps run it more frequently. And after this trimming, it's easy to identify basic blocks which will surely cause UB if jumped to (since their statement list is empty).

What do y'all think? cc @oli @centril

oli (Nov 14 2019 at 14:27, on Zulip):

I like it, this seems like a much more modular (and thus maintainable) approach

oli (Nov 14 2019 at 14:28, on Zulip):

and it automatically errors on the conservative side instead of silently doing bad optimizations

centril (Nov 14 2019 at 14:35, on Zulip):

Seems good

centril (Nov 14 2019 at 14:36, on Zulip):

My one question is how we roll this out since we have two separate PRs now

centril (Nov 14 2019 at 14:36, on Zulip):

should we land both and then do cleanup after or do it from the get-go?

rkruppe (Nov 14 2019 at 14:40, on Zulip):

I don't have a strong opinion on that.

centril (Nov 14 2019 at 14:41, on Zulip):

@oli please instruct :P

oli (Nov 14 2019 at 14:42, on Zulip):

I think we can just make the current PRs mir-opt-level=3

oli (Nov 14 2019 at 14:42, on Zulip):

then we can merge them (+ have an open issue about the unsoundness of the optimizations)

centril (Nov 14 2019 at 14:43, on Zulip):

@oli wouldn't be too difficult to filter out bbs with InlineAsm in my PR

centril (Nov 14 2019 at 14:43, on Zulip):

so I can make it sound from the start

oli (Nov 14 2019 at 14:43, on Zulip):

:shrug: as long as it has a FIXME with an issue

centril (Nov 14 2019 at 14:44, on Zulip):

:+1:

centril (Nov 14 2019 at 14:44, on Zulip):

@oli what are you thoughts on inlining and my optimization?

centril (Nov 14 2019 at 14:44, on Zulip):

(also, why aren't we inlining atm on release mode?)

oli (Nov 14 2019 at 15:09, on Zulip):

inlining is still broken

oli (Nov 14 2019 at 15:09, on Zulip):

you can't boostrap rustc

oli (Nov 14 2019 at 15:09, on Zulip):

but I don't have the capacity to tackle another project atm

oli (Nov 14 2019 at 15:09, on Zulip):

and I don't even have any tips to give someone who'd want to tackle it

centril (Nov 14 2019 at 15:14, on Zulip):

ouch; then my optimization should definitely be -Z mir-opt-level=3

Wesley Wiser (Nov 14 2019 at 15:22, on Zulip):

and I don't even have any tips to give someone who'd want to tackle it

I have been looking at this off and on but I'm currently sidetracked on const prop stuff. My previous approach was doing binary searches with the -Z fuel argument to find where the optimization was occurring. But that's very time intensive...

Last update: Dec 12 2019 at 00:45UTC