Stream: t-compiler/wg-mir-opt

Topic: removing () locals


Zoxc (Mar 14 2020 at 15:20, on Zulip):

Are there any plans which would get rid of all the () locals generated? I see they survive until LLVM IR which isn't great.

oli (Mar 14 2020 at 15:21, on Zulip):

Yes, but it's blocked on #67191 because otherwise we start not evaluating constants with () value even if the constant would error if evaluated

oli (Mar 14 2020 at 15:22, on Zulip):

We've actually had this optimization but had to roll back out of that reason.

Zoxc (Mar 14 2020 at 15:40, on Zulip):

Can't we just keep such constant assignments? What optimization was that?

oli (Mar 14 2020 at 15:42, on Zulip):

That's what we do: https://github.com/rust-lang/rust/blob/42ce9b4e9054d63c8f352f994e2d5dae7d04130f/src/librustc_mir/transform/simplify.rs#L375

Zoxc (Mar 14 2020 at 15:48, on Zulip):

SimplifyLocals doesn't remove all () locals though. Some are assigned by calls and those stick around. Maybe we'd want something that could purge all ZST locals?

oli (Mar 14 2020 at 15:57, on Zulip):

Hmm... we could replace all Rvalue::Ref to () locals by a constant that is essentially ptr::dangling(). But I don't see how we can get rid of function return places. They need to have a return place

Zoxc (Mar 14 2020 at 15:59, on Zulip):

We could just allow functions to return without a return place. So _3 = const print() -> [return: bb2, unwind: bb4] would be just const print() -> [return: bb2, unwind: bb4]. It would make MIR output more readable too =P

Zoxc (Mar 14 2020 at 16:00, on Zulip):

I don't think borrowing () is very common though, we could perhaps keep that. I think every non-returning call making it's own () local is the problematic case.

Zoxc (Mar 14 2020 at 16:05, on Zulip):

Maybe just removing StorageDead/Live for () locals would help with performance

eddyb (Mar 14 2020 at 16:46, on Zulip):

I wonder if that would interfere with anything @RalfJ wants to rely on Storage{Live,Dead} for

eddyb (Mar 14 2020 at 16:46, on Zulip):

but I generally agree we should strive for less noisy MIR

Zoxc (Mar 14 2020 at 16:56, on Zulip):

Miri shouldn't run on optimized MIR at least (if you want to catch UB)

eddyb (Mar 14 2020 at 17:58, on Zulip):

@oli ^^ impedance mismatch :P

oli (Mar 14 2020 at 19:05, on Zulip):

we can always overload the optimized_mir query if we want to (or set the mir opt level low)

bjorn3 (Mar 14 2020 at 19:10, on Zulip):

@oli Overloading the optimized_mir query is hard, as rustc_mir doesn't export any mir passes. Some mir passes are mandatory though, like the generator transform and borrowck.

oli (Mar 14 2020 at 19:11, on Zulip):

true, but at mir-opt-level=0 we should only be doing strictly necessary things

RalfJ (Mar 15 2020 at 10:23, on Zulip):

eddyb said:

I wonder if that would interfere with anything RalfJ wants to rely on Storage{Live,Dead} for

I am not relying on StorageLive/Dead for anything, really. We are just interpreting them in Miri, giving them concrete meaning, and I am trying to align dataflow analyses assumptions with the meaning they are given there.

RalfJ (Mar 15 2020 at 10:24, on Zulip):

I assume with mir-opt-level=0, none of these optimizations happen? That would be good for Miri's ability to still check that the initially created MIR makes sense.

Wesley Wiser (Mar 16 2020 at 14:52, on Zulip):

Yes, that definitely should be the case.

Last update: Apr 03 2020 at 18:25UTC