Stream: t-compiler/wg-mir-opt

Topic: Debug vs Release opts


Félix Fischer (May 22 2020 at 15:46, on Zulip):

Correct me if I'm wrong here.
My understanding is that currently, we have 3 "levels" of optimizations in MIR:

What I think we could use, is having an enum-based opt mode instead of a level-based opt mode:

Debug would more or less reflect our current Level 1. Release would allow us to start using optimizations that improve compilation times in release mode, like inlining (cc @Wesley Wiser, you have experience in this example case :3). It would also allow us to use many opts that are currently chilling on level 2 or 3, which are sound and stable but otherwise not useable in Debug because the CGU partitioning scheme makes them a compile-time regression on Debug builds.

Félix Fischer (May 22 2020 at 15:49, on Zulip):

CC @oli

Jonas Schievink (May 22 2020 at 15:53, on Zulip):

Note that rustc doesn't have a --release mode, it has opt-level=0,1,2,3,s,z instead

Félix Fischer (May 22 2020 at 15:54, on Zulip):

Oh really? Lol, I'm clearly using cargo too much :laughter_tears:

Félix Fischer (May 22 2020 at 15:54, on Zulip):

Still, don't people mainly use cargo? What's up with opt-levels being different in rustc and cargo? That's kinda weird

Jonas Schievink (May 22 2020 at 15:55, on Zulip):

opt-levels are configurable in Cargo, it just defaults to 0 and 3 for the dev and release profiles, respectively

Jonas Schievink (May 22 2020 at 15:56, on Zulip):

To some degree I guess it's something rustc inherited from C compilers

Félix Fischer (May 22 2020 at 15:56, on Zulip):

Yeah, it feels a lot that way :3

Félix Fischer (May 22 2020 at 15:57, on Zulip):

That feels very gcc -OX ...

Félix Fischer (May 22 2020 at 15:59, on Zulip):

Kay, updated the original post to reflect that. Thank you Jonas :)

eddyb (May 23 2020 at 11:46, on Zulip):

it is GCC/Clang -O

eddyb (May 23 2020 at 11:47, on Zulip):

idk why we call the "dev" Cargo profile "Debug mode" colloquially :P

eddyb (May 23 2020 at 11:48, on Zulip):

dev/release is inherited from... other language package managers, I'm guessing? it's not a compiler concept, although there is some implication that you'd want optimizations in release mode

eddyb (May 23 2020 at 11:48, on Zulip):

but something likely more meaningful is how debug! logging w/ the log crate is disabled in release builds

eddyb (May 23 2020 at 11:49, on Zulip):

(same with debug_assert!, which I keep forgetting exists)

eddyb (May 23 2020 at 11:49, on Zulip):

you can think of that "you're releasing this to end-users, so you want it to be fast and it doesn't necessarily need to be debuggable"

eddyb (May 23 2020 at 11:50, on Zulip):

(but as @Jonas Schievink mentioned, you can control both opt levels and debuginfo and I think maybe even debug assertions through Cargo profiles. there's no custom profiles... yet, although I know I few places which direly need them)

eddyb (May 23 2020 at 11:52, on Zulip):

/me is somewhat opposed to a debug/release optimization split, especially if debuginfo is involved, since I want to preserve debuginfo through as many optimizations as possible

eddyb (May 23 2020 at 11:53, on Zulip):

@Félix Fischer however, I do like the unstable/unsound and compile-time vs run-time distinctions

eddyb (May 23 2020 at 11:53, on Zulip):

there should probably be a clear dedicated opt-in for "known to be unsound"

eddyb (May 23 2020 at 11:55, on Zulip):

frankly that's all we need to do, and maybe remove level -Zmir-opt-level=3?

eddyb (May 23 2020 at 11:57, on Zulip):

as for using an enum, it could be this:

enum MirOptLevel {
    Disabled = 0,
    Default = 1,
    Aggressive = 2,
}
bjorn3 (May 23 2020 at 11:59, on Zulip):

Can we also have a flag to specify the optimizations to run rather than choose a fixed combination? For example enable const-prop and inlining, but not copy-prop.

Félix Fischer (May 23 2020 at 18:28, on Zulip):

@bjorn3 sure. That makes a lot of sense, specially for testing and for keeping the optimization suite as a whole sound over time.

Félix Fischer (May 23 2020 at 18:31, on Zulip):

(@eddyb)

/me is somewhat opposed to a debug/release optimization split, especially if debuginfo is involved, since I want to preserve debuginfo through as many optimizations as possible

I agree with you here, which makes me think that maybe we want a third, different compilation mode that's just for getting that extra 0.5% of performance out of the program:

RalfJ (May 24 2020 at 07:49, on Zulip):

eddyb said:

there should probably be a clear dedicated opt-in for "known to be unsound"

really? IMO "known to be unsound" optimizations should just not happen, period. when we find a soundness issue, we either fix it or remove the offending optimization.

Félix Fischer (May 24 2020 at 19:02, on Zulip):

^

Félix Fischer (May 24 2020 at 19:04, on Zulip):

I mean... I'm imagining that just as there are clippy lints that are not really that great but some people want anyway, because they work for their setup or whatever, someone could want an optimization that happens to work soundly for their use case.

Félix Fischer (May 24 2020 at 19:04, on Zulip):

However there's a big difference

Félix Fischer (May 24 2020 at 19:04, on Zulip):

If a lint stops working for you, your code is rejected and you see it

Félix Fischer (May 24 2020 at 19:04, on Zulip):

That's the failure mode

Félix Fischer (May 24 2020 at 19:05, on Zulip):

Whereas if an optimization starts breaking your code, you'll not notice right away. That's a bad failure mode

Félix Fischer (May 24 2020 at 19:06, on Zulip):

I also would personally prefer to not inherit this thing which is very common in C compilers, which is that optimizations break perfectly sound code. That always sucks to debug. I would love it if we never did that to people, or at least not knowingly.

Félix Fischer (May 24 2020 at 19:07, on Zulip):

I mean... llvm does that to us already! We cannot use noalias because optimizations break it

Félix Fischer (May 24 2020 at 19:08, on Zulip):

I would much prefer it if we stopped this tradition of optimizations breaking people's code

Félix Fischer (May 24 2020 at 19:08, on Zulip):

:P

RalfJ (May 24 2020 at 19:57, on Zulip):

Félix Fischer said:

I would much prefer it if we stopped this tradition of optimizations breaking people's code

I can't even say how much I agree with this.^^

RalfJ (May 24 2020 at 19:58, on Zulip):

Somehow an entire industry came to accept that -O3 sometimes breaks your code and that's just what optimizations do. (granted, most of those are probably cases of UB in the program, but there are also more than enough compiler bugs going around.) I don't know how we ended up there, but it's totally awful and really needs fixing. plus, Rust is a safe language, so it should care even more about fixing that.

Hanna Kruppe (May 24 2020 at 20:55, on Zulip):

I mean, telling users "oh yeah, optimization X has soundness bugs but you can pass -Zwhatever to enable it anyway, it'll probably make your code faster" would be a bad idea, duh. But in a situation where we know of a soundness issue in a MIR optimization and can't fix it promptly, I don't think "remove the optimization" is the best alternative. Removing the code (or at least, any way to run it) just makes it harder to work on a fix: you'd have to revert the removal and then fix up any bitrot that occurred in the mean time before you can even check if a test case for the unsoundness still reproduces. Leaving the optimization available behind a scary hidden flag also avoids miscompiling people's programs, but any existing tests for the pass keep running, mechanical refactors get applied to it, etc. and so development gets easier.

BTW, this isn't too different from "incomplete feature gates", some of which also have soundness issues but we keep them in-tree to enable iterative development and testing.

Félix Fischer (May 25 2020 at 00:29, on Zulip):

Yep, @Hanna Kruppe, I think I completely agree with you. I think something that's really good about how we work right now, is that all mir optimizations have more or less their own test suite that runs on the CI pipelines for rustc.

Félix Fischer (May 25 2020 at 00:31, on Zulip):

I would leave them summoneable through some scary flag, just as we do today. Something like -Zwhatever I think is fine.

Félix Fischer (May 25 2020 at 00:32, on Zulip):

I would definitely never allow unsound opts to get into the stable compiler though

Félix Fischer (May 25 2020 at 00:33, on Zulip):

And also that enum that @eddyb suggested, I would also change. From:

enum MirOptLevel {
    Disabled = 0,
    Default = 1,
    Aggressive = 2,
}

To:

enum MirOptLevel {
    Disabled = 0,
    Light = 1,
    Heavy = 2,
    Unsound = 3,
}
Last update: Sep 28 2020 at 16:15UTC