Stream: t-compiler/wg-mir-opt

Topic: how to trigger MIR inliner at will


eddyb (Feb 03 2020 at 14:55, on Zulip):

I'm trying to fix https://github.com/rust-lang/rust/issues/67586 and step 0 is to make a self-contained reproduction, presumably relying on MIR inlining

eddyb (Feb 03 2020 at 14:55, on Zulip):

but this doesn't do it https://godbolt.org/z/fgS-hj

eddyb (Feb 03 2020 at 14:56, on Zulip):

ftr, it's this snippet, using -Zmir-opt-level=2 --emit=mir -Cdebuginfo=2:

#[inline(always)]
fn foo(x: usize) -> usize {
    x
}

pub fn bar(y: usize) -> usize {
    foo(y)
}
oli (Feb 03 2020 at 14:56, on Zulip):

this works for me locally

eddyb (Feb 03 2020 at 14:56, on Zulip):

even with --emit=mir?

oli (Feb 03 2020 at 14:56, on Zulip):

haven't tested

oli (Feb 03 2020 at 14:56, on Zulip):

but with -Zdump-mir=all it is

// MIR for `foo`
// source = MirSource { instance: Item(DefId(0:3 ~ foo[8787]::foo[0])), promoted: None }
// pass_name = Inline
// disambiguator = before

fn  foo(_1: usize) -> usize {
    debug x => _1;                       // in scope 0 at foo.rs:2:8: 2:9
    let mut _0: usize;                   // return place in scope 0 at foo.rs:2:21: 2:26

    bb0: {
        _0 = _1;                         // bb0[0]: scope 0 at foo.rs:3:5: 3:6
        return;                          // bb0[1]: scope 0 at foo.rs:4:2: 4:2
    }
}
oli (Feb 03 2020 at 14:57, on Zulip):

hmm this is before inlining !?

eddyb (Feb 03 2020 at 14:57, on Zulip):

bar is the one that foo should get inlined into, you can ignore foo

oli (Feb 03 2020 at 14:57, on Zulip):

rofl

oli (Feb 03 2020 at 14:57, on Zulip):

ups

oli (Feb 03 2020 at 14:57, on Zulip):

ok, doesn't work either

oli (Feb 03 2020 at 14:58, on Zulip):

I'm guessing the weight algorithm is broken for inline-always

eddyb (Feb 03 2020 at 14:58, on Zulip):

@andjo403 @Wesley Wiser do you know anything weird about that the inliner that could cause this?

eddyb (Feb 03 2020 at 14:58, on Zulip):

@oli I mean, it doesn't work without it or with just #[inline] either

eddyb (Feb 03 2020 at 14:59, on Zulip):

I should go and read an inline mir-opt test, lol

eddyb (Feb 03 2020 at 15:02, on Zulip):

you have to be kidding me... it's sensitive to the declaration order?!

eddyb (Feb 03 2020 at 15:04, on Zulip):

this works https://godbolt.org/z/XerSJ8

oli (Feb 03 2020 at 15:05, on Zulip):

https://github.com/rust-lang/rust/blob/c58e09f138075ce6b3079f41f9c2f192a15b896c/src/librustc_mir/transform/inline.rs#L112

eddyb (Feb 03 2020 at 15:06, on Zulip):

this is so terrible Q_Q

eddyb (Feb 03 2020 at 15:07, on Zulip):

I should've just done this from the start: https://godbolt.org/z/6JP9oR

eddyb (Feb 03 2020 at 15:08, on Zulip):

@oli how is that not a big issue perma-open until we find a real solution?

eddyb (Feb 03 2020 at 15:09, on Zulip):
    debug foo_arg => _1;                 // in scope 0 at ./example.rs:1:12: 1:19
    scope 1 {
        debug x => _1;                   // in scope 1 at /rustc/9ed29b6ff6aa2e048b09c27af8f62ee3040bdb37/src/libcore/convert/mod.rs:106:26: 106:27
    }
eddyb (Feb 03 2020 at 15:09, on Zulip):

this is what I was after, it does trigger a LLVM assertion, and it should be trivial to fix

oli (Feb 03 2020 at 15:13, on Zulip):

@eddyb I have a checkout open working on it

eddyb (Feb 03 2020 at 15:13, on Zulip):

working on what?

oli (Feb 03 2020 at 15:13, on Zulip):

the protoype is quite invasive though

oli (Feb 03 2020 at 15:14, on Zulip):

I'm working on a real solution on the "don't cause cycle errors in the inliner"

eddyb (Feb 03 2020 at 15:14, on Zulip):

if you mean the inliner, the problem is a design one, not implementation

oli (Feb 03 2020 at 15:14, on Zulip):

how so?

eddyb (Feb 03 2020 at 15:15, on Zulip):

cycles in the query engine are not sound

eddyb (Feb 03 2020 at 15:15, on Zulip):

because for any non-trivial cycle, you have N "mutually recursive" queries, and depending which one is entered, the result of all of them will differ

oli (Feb 03 2020 at 15:21, on Zulip):

I'm not touching the query engine

oli (Feb 03 2020 at 15:22, on Zulip):

I think I have a solution completely side stepping any kind of issues

eddyb (Feb 03 2020 at 15:22, on Zulip):

I kind of doubt it, given everything I've heard about this before

eddyb (Feb 03 2020 at 15:23, on Zulip):

so talk to @mw and @nikomatsakis (to be clear, I want to be impressed :P)

oli (Feb 03 2020 at 15:23, on Zulip):

I'm not saying it is a cheap or good solution, but a PoC giving us a starting point

Wesley Wiser (Feb 03 2020 at 15:48, on Zulip):

you have to be kidding me... it's sensitive to the declaration order?!

Yes. You have no idea how many times I've done this exact thing trying to get the inliner to trigger only for the declaration order to be what's preventing it.

eddyb (Feb 03 2020 at 15:51, on Zulip):

@oli I just mean that this is a known design problem with no solutions that we know of that work at all

oli (Feb 03 2020 at 15:51, on Zulip):

Let me be on my "I solved this"-high for a bit longer :P rebasing rn

eddyb (Feb 03 2020 at 16:11, on Zulip):

can I see what rustc invocations compiletest does?!

bjorn3 (Feb 03 2020 at 16:14, on Zulip):

Maybe ./x.py test -vvv works?

Wesley Wiser (Feb 03 2020 at 16:15, on Zulip):

If the test fails, I'm pretty sure the test runner prints out the rustc arguments used

eddyb (Feb 03 2020 at 16:22, on Zulip):

the problem is it doesn't fail :(

eddyb (Feb 03 2020 at 16:22, on Zulip):

but thanks, I can add a syntax error

eddyb (Feb 03 2020 at 16:26, on Zulip):

@bjorn3 nope, that just spams more in rustbuild, nothing extra from compiletest compared to -vv

eddyb (Feb 03 2020 at 16:40, on Zulip):

ugh in a bin crate I can't force a function to be codegen'd without calling it from main, I really don't like the fragility there :/

Wesley Wiser (Feb 03 2020 at 16:40, on Zulip):

I'm not sure the context of what you're trying to do but I believe you can use --emit mir to force it

Wesley Wiser (Feb 03 2020 at 16:41, on Zulip):

But that might not work for what you're doing

eddyb (Feb 03 2020 at 16:41, on Zulip):

I need 1. MIR inlining 2. -C debuginfo=2 and 3. codegen to trigger this

Wesley Wiser (Feb 03 2020 at 16:41, on Zulip):

Ah, yeah that won't work then

eddyb (Feb 03 2020 at 16:41, on Zulip):

(it's a LLVM verification failure bug)

eddyb (Feb 03 2020 at 17:58, on Zulip):

btw this is the fix I was trying to test https://github.com/rust-lang/rust/pull/68802

eddyb (Feb 05 2020 at 04:45, on Zulip):

well played oli, you showed me https://github.com/rust-lang/rust/pull/68828#issuecomment-582235377

eddyb (Feb 05 2020 at 04:51, on Zulip):

you (accidentally?) unblocked something that predates the query system and its cycle detection (and any way to catch such a cycle)

eddyb (Feb 05 2020 at 04:57, on Zulip):

I am in awe and also ashamed I told you anything discouraging

eddyb (Feb 05 2020 at 04:58, on Zulip):

but also I wish we could get the callers on optimized_mir just before inlining. do we just agree to put inlining first?

eddyb (Feb 05 2020 at 04:58, on Zulip):

that is, inlining seeing validated_mir just like the relevant queries

eddyb (Feb 05 2020 at 04:59, on Zulip):

(also feel free to split this topic somehow, it's getting all kinds of offtopicy)

oli (Feb 05 2020 at 06:34, on Zulip):

My scheme can be updated to work just before inlining by splitting the mir opt query, that just seemed too intrusive for me

Last update: Sep 22 2020 at 02:30UTC