Stream: t-compiler/major changes

Topic: -Zmir-opt-level Reform compiler-team#319


triagebot (Jun 23 2020 at 13:50, on Zulip):

A new proposal has been announced: #319. It will be brought up at the next meeting.

bjorn3 (Jun 23 2020 at 14:24, on Zulip):

The proposal has a level for less debug+little comp time cost and one for less debug+bigger comp time cost. What if you want to enable optimizations that have a big compile time cost, but you do want full debugability?

Wesley Wiser (Jun 23 2020 at 14:42, on Zulip):

As an example, what I had in mind were things like this:

const fn add(x: usize, y: usize) -> usize {
    x + y
}

fn main() {
    let z = add(1, 2);
}

where under heavy optimization, this might become the equivalent of:

fn main() {
    let z = 3;
}

But it's not as debuggable because you can't step into the add invocation or put a breakpoint on line 2. You can still have full debug symbols and line information, but it's not the same experience you might hope for without the aggressive optimizations enabled.

Jonas Schievink (Jun 23 2020 at 14:56, on Zulip):

I'd prefer if -Zunsound-mir-optimizations would be called -Zexperimental-mir-optimizations, or something to that extent, since IMO it is more useful to have it as a sort of staging area for new optimizations, where we might not be sure how well they work in practice, but they don't have any acute known soundness issues.

Wesley Wiser (Jun 23 2020 at 15:02, on Zulip):

Yeah, I like that a lot better. Great suggestion!

lqd (Jun 23 2020 at 15:07, on Zulip):

are the two flags important to be able to, say, have an experimental optimization at level 0, rather than have -Zmir-opt-level=experimental ?

Wesley Wiser (Jun 23 2020 at 15:10, on Zulip):

I think the stuff we know is broken or whatever should be gated separately from the stuff that works, but might be bad or compile times or whatever.

Wesley Wiser (Jun 23 2020 at 15:10, on Zulip):

So I was leaning toward two flags for social/documentation reasons not technical ones.

lqd (Jun 23 2020 at 15:11, on Zulip):

understood

Wesley Wiser (Jun 23 2020 at 15:11, on Zulip):

My impression is that some users out there will just go -O9999 if you let them

Wesley Wiser (Jun 23 2020 at 15:11, on Zulip):

-Zexperimental-mir-optimizations to me is kind of like a feature flag

Wesley Wiser (Jun 23 2020 at 15:12, on Zulip):

But there's really no reason ever for end-users to set it

lqd (Jun 23 2020 at 15:12, on Zulip):

yeah :) quite a number of papers have -O9 or more

Wesley Wiser (Jun 23 2020 at 15:12, on Zulip):

:face_palm:

Jonas Schievink (Jun 23 2020 at 15:13, on Zulip):

bjorn3 said:

The proposal has a level for less debug+little comp time cost and one for less debug+bigger comp time cost. What if you want to enable optimizations that have a big compile time cost, but you do want full debugability?

Given that we don't really have an equivalent to GCC's -Og with LLVM, I think it's reasonably to punt on this for MIR optimizations as well, at least for now.

lqd (Jun 23 2020 at 15:13, on Zulip):

(your earlier example was also very interesting, wrt the planned story around users' ability to develop/debug const fns which I don't see being mentioned often I don't think, or might have missed but that's another topic altogether)

Jonas Schievink (Jun 23 2020 at 15:15, on Zulip):

I suppose at some point after this we'd turn on -Zmir-opt-level=2 when -Copt-level=2/3 is passed, right?

Wesley Wiser (Jun 23 2020 at 15:16, on Zulip):

Yeah, that seems reasonable to me but I didn't want to suggest a ton of changes all in the same proposal.

Wesley Wiser (Jun 23 2020 at 15:17, on Zulip):

If we decide to go this way, it seems like there's also the potential for having an official stabilization path for new MIR optimizations.

Wesley Wiser (Jun 23 2020 at 15:17, on Zulip):

But, again, "Future Work" :slight_smile:

triagebot (Jun 23 2020 at 15:19, on Zulip):

@T-compiler: Proposal #319 has been seconded, and will be approved in 10 days if no objections are raised.

Josh Triplett (Jun 23 2020 at 16:47, on Zulip):

Seems like perhaps we should have an opt-level=g option.

RalfJ (Jun 23 2020 at 17:13, on Zulip):

I support this, expect I dont like "experimental" as a euphemism for "buggy"

RalfJ (Jun 23 2020 at 17:13, on Zulip):

I think the flag to enable known-incorrect optimizations should contain a trigger word like "broken", "wrong", "incorrect", "unsound" or so

RalfJ (Jun 23 2020 at 17:14, on Zulip):

Jonas Schievink said:

I'd prefer if -Zunsound-mir-optimizations would be called -Zexperimental-mir-optimizations, or something to that extent, since IMO it is more useful to have it as a sort of staging area for new optimizations, where we might not be sure how well they work in practice, but they don't have any acute known soundness issues.

the thing is, we have some passes with known accute soundness issues

RalfJ (Jun 23 2020 at 17:14, on Zulip):

what do you suggest we do with those?

RalfJ (Jun 23 2020 at 17:15, on Zulip):

see e.g. https://github.com/rust-lang/rust/pull/73262

Jonas Schievink (Jun 23 2020 at 17:21, on Zulip):

I expect that not all optimizations controlled by the flag will have known soundness issues, some optimizations might just be complex and new so we aren't yet confident that they're correct in practice (eg. the destination propagation pass once it's merged). For those it would make little sense to label them all as "unsound" off the bat.

(Perhaps the flag should take a pass name and only enable that pass, to make it easier to test out a specific experimental pass and avoid the known-unsound ones?)

RalfJ (Jun 23 2020 at 17:46, on Zulip):

I expect that not all optimizations controlled by the flag will have known soundness issues, some optimizations might just be complex and new so we aren't yet confident that they're correct in practice (eg. the destination propagation pass once it's merged). For those it would make little sense to label them all as "unsound" off the bat.

that sounds like we have 2 different usecases: a "holding area" for known-unsound optimizations, and a "staging area" for new optimizations

RalfJ (Jun 23 2020 at 17:47, on Zulip):

they might warrant separate treatment

Wesley Wiser (Jun 23 2020 at 17:54, on Zulip):

Thanks for the feedback @RalfJ!

Wesley Wiser (Jun 23 2020 at 17:55, on Zulip):

It feels to me like in practice, those two different flags would be used in very similar ways.

Wesley Wiser (Jun 23 2020 at 17:56, on Zulip):

I suppose it doesn't cost us very much to have two flags but it does feel a bit redundant.

Wesley Wiser (Jun 23 2020 at 17:59, on Zulip):

Is your concern that users won't know that this flag could cause misoptimizations/unsound behavior and just turn it on to get more optimization?

Wesley Wiser (Jun 23 2020 at 18:00, on Zulip):

We could emit a warning if this flag is used that explains it enables "experimental, buggy and potentially unsound optimizations that may break your code" or something to that effect.

RalfJ (Jun 23 2020 at 18:13, on Zulip):

Wesley Wiser said:

Is your concern that users won't know that this flag could cause misoptimizations/unsound behavior and just turn it on to get more optimization?

essentially, yes

RalfJ (Jun 23 2020 at 18:13, on Zulip):

Wesley Wiser said:

We could emit a warning if this flag is used that explains it enables "experimental, buggy and potentially unsound optimizations that may break your code" or something to that effect.

hm. I guess there is precedent for that with the "incomplete features" lint.

RalfJ (Jun 23 2020 at 18:14, on Zulip):

I personally prefer to be as aggressive as possible about this, with a really scary flag name, but I could see how people disagree with that ;)

Jonas Schievink (Jun 23 2020 at 18:17, on Zulip):

Maybe the flag should require a pass name to enable? Would make it impossible to enable multiple passes, but normally developers will likely only need to test one pass, unless I'm mistaken.

bjorn3 (Jun 23 2020 at 18:56, on Zulip):

Maybe add a -Zmir-passes option that is like -Cpasses except for MIR optimizations instead of LLVM optimizations?

Jonas Schievink (Jun 23 2020 at 19:15, on Zulip):

I was thinking of building a "proper" MIR pass manager, but that seems like it'd have much bigger scope than what's proposed here

Jonas Schievink (Jun 23 2020 at 19:16, on Zulip):

(I'm hoping something like that will fall out of our Sealed Rust work eventually)

Wesley Wiser (Jun 23 2020 at 19:40, on Zulip):

Jonas Schievink said:

Maybe the flag should require a pass name to enable?

I thought about that but the primary reason I wanted this flag was so we could keep using the mir-opt tests with our unstable/buggy/unsound/experimental optimizations and those tests already test against the output of specific passes. So it seemed fine to just make a binary on/off flag.

nikomatsakis (Jun 24 2020 at 15:05, on Zulip):

maybe "incomplete" optimizations?

RalfJ (Jun 24 2020 at 16:18, on Zulip):

"dangerous"?

nagisa (Jun 28 2020 at 21:03, on Zulip):

I'd rather not have a new flag, but since it'd be unstable, I guess I don’t care strongly either way… as long as its not something we would ever consider stabilising.

nagisa (Jun 28 2020 at 21:03, on Zulip):

Also, turns out you cannot post a reploy without subscribing to the stream – interesting!

nagisa (Jun 28 2020 at 21:04, on Zulip):

(Ability to turn on and off separate passes have many benefits, and is ultimately most flexible)

nagisa (Jun 28 2020 at 21:05, on Zulip):

(but as been mentioned above, it'd also involve most engineering effort, and managing optimisation ordering is a hard problem)

Wesley Wiser (Jun 29 2020 at 01:25, on Zulip):

nagisa said:

I'd rather not have a new flag, but since it'd be unstable, I guess I don’t care strongly either way… as long as its not something we would ever consider stabilising.

Yes, I completely agree. The flag would explicitly be permanently unstable.

RalfJ (Jul 05 2020 at 07:13, on Zulip):

even with per-pass control I feel like there should be a speed bump to enabling a pass that is considered broken. but that could be in the pass name (-Zmir-opt-pass=simplify-match-arms_unsound etc)

Félix Fischer (Jul 06 2020 at 08:03, on Zulip):

Omg I almost missed this discussion! I'm glad I'm here.

Félix Fischer (Jul 06 2020 at 08:03, on Zulip):

I have two cents

Félix Fischer (Jul 06 2020 at 08:04, on Zulip):

First: I agree with Ralf in that the "experimental" (read: many of these are still broken) opts should have a speedbump or two in the compiler UI

Félix Fischer (Jul 06 2020 at 08:05, on Zulip):

I would use a harsh-ish word at least in the flag

Félix Fischer (Jul 06 2020 at 08:06, on Zulip):

Also, wait, tangent: thank you @Wesley Wiser for making this proposal. I had this in my mind after the discussion regarding mir opt levels but never took the time to continue with it. Thank you thank you thank you.

Félix Fischer (Jul 06 2020 at 08:07, on Zulip):

Second: Wesley, I might be misreading the proposal, but... are all the current Level 2 optimizations classified as being broken?

Félix Fischer (Jul 06 2020 at 08:07, on Zulip):

Because I know for a fact that there's one that isn't broken but that we gated at Level 2!

Félix Fischer (Jul 06 2020 at 08:09, on Zulip):

(That small thing that we did together to const-propagate function parameters, do you remember?)

Félix Fischer (Jul 06 2020 at 08:10, on Zulip):

(We gated that at level 2 because it gave a performance regression from the codegen units that rustc sends to llvm)

Wesley Wiser (Jul 06 2020 at 14:48, on Zulip):

Yeah, I think the only 1:1 correspondence we have right now is the mir-opt-level=1 is our stable, performant optimizations. Part of implementing this MCP will involve figuring out what mir-opt-level our other optimizations should go into and also gating some behind the unstable/experimental/unsound flag.

Félix Fischer (Jul 06 2020 at 18:43, on Zulip):

Great to hear you have that in mind :blush:

Félix Fischer (Jul 06 2020 at 18:45, on Zulip):

Ping me whenever that initiative gets started so that I can help :3

triagebot (Jul 08 2020 at 14:00, on Zulip):

This proposal has been accepted: #319.

RalfJ (Jul 08 2020 at 14:46, on Zulip):

okay so the MCP has been closed, but what does that mean for the still open discussion around what to call the flag(s) to enable unfinishd or known-broken optimizations?

Wesley Wiser (Jul 08 2020 at 15:15, on Zulip):

@RalfJ I was planning to leave that discussion to the PR which adds said flag(s). Does that seem reasonable to you?

RalfJ (Jul 10 2020 at 15:08, on Zulip):

yeah that seems fair

Last update: May 07 2021 at 07:45UTC