Stream: t-compiler

Topic: MIR pass ordering


RalfJ (Nov 20 2018 at 13:29, on Zulip):

Is it expected that lowering generators to state machines happens after inlining? I'd think MIR generation should be basically done before we do any interesting optimizations, and this lowering seems to conceptually be part of MIR construction.

RalfJ (Nov 20 2018 at 13:43, on Zulip):

and then we have the deaggregator turning things like

_1 = [generator@test.rs:6:25: 15:6] { $state: const 0u32 };

into

(_1.0: u32) = const 0u32;

which is a problem because now only the first field of the struct actually gets initialized...

RalfJ (Nov 20 2018 at 13:44, on Zulip):

so the Retag(_1) that got inserted right after this sees uninitialized data

RalfJ (Nov 20 2018 at 13:49, on Zulip):

hm, actually none of this pass ordering can possibly help

RalfJ (Nov 20 2018 at 13:50, on Zulip):

would it be reasonable to say that the fields of a generator (all but the first, with the state) are implicitly wrapped in a MaybeUninit?

RalfJ (Nov 20 2018 at 13:50, on Zulip):

because that seems how things are actually being used...

oli (Nov 20 2018 at 14:08, on Zulip):

yea, that sounds about like how I understand generators work

nikomatsakis (Nov 20 2018 at 14:40, on Zulip):

Is it expected that lowering generators to state machines happens after inlining? I'd think MIR generation should be basically done before we do any interesting optimizations, and this lowering seems to conceptually be part of MIR construction.

sounds wrong

nikomatsakis (Nov 20 2018 at 14:40, on Zulip):

would it be reasonable to say that the fields of a generator (all but the first, with the state) are implicitly wrapped in a MaybeUninit?

but yes, sounds right

oli (Nov 20 2018 at 14:43, on Zulip):

PR with a workaround/fix for miri: https://github.com/rust-lang/rust/pull/56100

nikomatsakis (Nov 20 2018 at 14:47, on Zulip):

yeah, I saw it

RalfJ (Nov 20 2018 at 15:37, on Zulip):

sounds wrong

seems like that got introduced with https://github.com/rust-lang/rust/commit/6e5dacbd5e8aab43cdc4c2f1d4ec66fe0b8d4bed by @eddyb

RalfJ (Nov 20 2018 at 15:37, on Zulip):

well, actually before that the pass was even later^^

RalfJ (Nov 20 2018 at 15:41, on Zulip):

this goes back all the way to the initial commit https://github.com/rust-lang/rust/commit/d861982ca6a1fa5773373362771aa08b9f732de0#diff-698522bf32a92da4331b4bd3f6ed31c6 by @Zoxc. Should I, uh, file an issue or so?

eddyb (Nov 20 2018 at 15:47, on Zulip):

I moved StateTransform earlier because it has some asserts that break

eddyb (Nov 20 2018 at 15:47, on Zulip):

but the intention is to do it as late as possible so we optimize generators

eddyb (Nov 20 2018 at 15:48, on Zulip):

including inlining generators

eddyb (Nov 20 2018 at 15:48, on Zulip):

I think maybe the asserts should be done early and the StateTransform moved late

RalfJ (Nov 20 2018 at 16:01, on Zulip):

including inlining generators

but can't we better inline them after generator lowering because then they are just normal functions?

RalfJ (Nov 20 2018 at 16:01, on Zulip):

currently inlining has to know how to special-case generators

eddyb (Nov 20 2018 at 16:01, on Zulip):

no, because the normal functions are already state-transformed

RalfJ (Nov 20 2018 at 16:01, on Zulip):

yes that's exactly what I am saying

eddyb (Nov 20 2018 at 16:02, on Zulip):

if you inline after, you can't merge the state

RalfJ (Nov 20 2018 at 16:02, on Zulip):

"merge"?

eddyb (Nov 20 2018 at 16:02, on Zulip):

flatten

eddyb (Nov 20 2018 at 16:02, on Zulip):

not have nested state tags

RalfJ (Nov 20 2018 at 16:02, on Zulip):

oh

RalfJ (Nov 20 2018 at 16:02, on Zulip):

I see

eddyb (Nov 20 2018 at 16:02, on Zulip):

now I don't think inlining is properly set up for this

RalfJ (Nov 20 2018 at 16:03, on Zulip):

it probably isn't^^

eddyb (Nov 20 2018 at 16:03, on Zulip):

it would need to query the untransformed generators, and optimized_mir is after the transformation

RalfJ (Nov 20 2018 at 16:03, on Zulip):

hm. this pretty much means a formal spec for MIR has to spec generators

RalfJ (Nov 20 2018 at 16:03, on Zulip):

however, given that borrowck happens before generator lowering, that might be better anyway

eddyb (Nov 20 2018 at 16:03, on Zulip):

that's already true and the surface area is pretty small

eddyb (Nov 20 2018 at 16:03, on Zulip):

yeah, generators have a special relationship with borrowck

eddyb (Nov 20 2018 at 16:04, on Zulip):

that's the best reason for understanding all of this state stuff before LLVM IR

eddyb (Nov 20 2018 at 16:04, on Zulip):

I guess that and guaranteed unboxing

RalfJ (Nov 20 2018 at 16:04, on Zulip):

if it happens after borrowck anyway, why care if it is done in MIR or LLVM?

RalfJ (Nov 20 2018 at 16:05, on Zulip):

(miri is happy that it's done in MIR though because it means it doesn't have to care^^)

eddyb (Nov 20 2018 at 16:05, on Zulip):

wait I was confusing

eddyb (Nov 20 2018 at 16:06, on Zulip):

what I mean is having any sort of language-level understanding of generators

RalfJ (Nov 20 2018 at 16:06, on Zulip):

yeah I agree with that

eddyb (Nov 20 2018 at 16:06, on Zulip):

C++ can leave it all to LLVM because it doesn't need to type-check based on state that's live across yields

eddyb (Nov 20 2018 at 16:06, on Zulip):

or anything like that

RalfJ (Nov 20 2018 at 16:07, on Zulip):

hm yeah

RalfJ (Nov 20 2018 at 16:07, on Zulip):

I will wonder how to best think about MIR with and without the extra statements I introduced. basically currently we have one syntax for two languages

eddyb (Nov 20 2018 at 16:07, on Zulip):

heh

RalfJ (Nov 20 2018 at 16:07, on Zulip):

well, "syntax"

RalfJ (Nov 20 2018 at 16:08, on Zulip):

before Retag is added, the semantics implicitly does that stuff, while after introducing these things it is explicit. or so.

RalfJ (Nov 20 2018 at 16:08, on Zulip):

there's some danger here of considering the "wrong" one so it might be worth generating them early some day, like, together with initial MIR building? or reflecting this change in semantics in the type somehow? Mir<ImplicitRetag> or so...

eddyb (Nov 20 2018 at 16:09, on Zulip):

there might be an interesting angle here where lowering to a different IR, that maybe loses some of the typedness of MIR, adds this information

eddyb (Nov 20 2018 at 16:09, on Zulip):

so you convert complexity instead of strictly adding

RalfJ (Nov 20 2018 at 16:10, on Zulip):

a twist of that is to use the same syntax/datatype for all these IRs

RalfJ (Nov 20 2018 at 16:10, on Zulip):

"compilation/lowering to a subset" or so, I saw that somewhere recently

RalfJ (Nov 20 2018 at 16:10, on Zulip):

generators actually do that -- they lower from full MIR to MIR\Generators

eddyb (Nov 20 2018 at 16:13, on Zulip):

Cranelift also uses the same IR for the higher-level input and for the heavily transformed version that contains e.g. register allocation, spills, expanded operations when the original didn't have a native encoding on the target, etc., and then it does a straight-forward instruction selection pass on the result, without having to use target-specific instructions for most things

eddyb (Nov 20 2018 at 16:13, on Zulip):

miri is, kinda interestingly, on-the-fly monomorphization and lowering to more primitive ops, and execution of those ops

eddyb (Nov 20 2018 at 16:15, on Zulip):

one could split those up, into something that looks like a codegen backend, and a separate "abstract memory machine"

eddyb (Nov 20 2018 at 16:16, on Zulip):

LLVM IR is gnarly, but e.g. Cranelift IR could be emulated on the lower half of miri just fine!

eddyb (Nov 20 2018 at 16:16, on Zulip):

/me pokes @Dan Gohman

nikomatsakis (Nov 20 2018 at 18:15, on Zulip):

I will wonder how to best think about MIR with and without the extra statements I introduced. basically currently we have one syntax for two languages

this has been true for a long time, I've been wondering if we want to make this split more explicit

nikomatsakis (Nov 20 2018 at 18:15, on Zulip):

(but I'm sort of inclined not to yet)

Jake Goulding (Nov 21 2018 at 02:40, on Zulip):

Time for HMIR, MMIR, and LMIR

Last update: Nov 20 2019 at 01:30UTC