Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unused MIR statement directly before return #70531

Closed
bjorn3 opened this issue Mar 29, 2020 · 2 comments · Fixed by #70595
Closed

Unused MIR statement directly before return #70531

bjorn3 opened this issue Mar 29, 2020 · 2 comments · Fixed by #70595
Assignees
Labels
A-cranelift Things relevant to the [future] cranelift backend A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt Area: MIR optimizations C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@bjorn3
Copy link
Member

bjorn3 commented Mar 29, 2020

$ rustc foo.rs --crate-type lib --emit mir -Cpanic=abort
fn map(x: Option<Box<()>>) -> Option<Box<()>> {
    match x {
        None => None,
        Some(x) => Some(x),
    }
}
// WARNING: This output format is intended for human consumers only
// and is subject to change without notice. Knock yourself out.
fn  map(_1: std::option::Option<std::boxed::Box<()>>) -> std::option::Option<std::boxed::Box<()>> {
    debug x => _1;                       // in scope 0 at a.rs:1:8: 1:9
    let mut _0: std::option::Option<std::boxed::Box<()>>; // return place in scope 0 at a.rs:1:31: 1:46
    let mut _2: isize;                   // in scope 0 at a.rs:3:9: 3:13
    let _3: std::boxed::Box<()>;         // in scope 0 at a.rs:4:14: 4:15
    let mut _4: std::boxed::Box<()>;     // in scope 0 at a.rs:4:25: 4:26
    let mut _5: isize;                   // in scope 0 at a.rs:6:1: 6:2
    scope 1 {
        debug x => _3;                   // in scope 1 at a.rs:4:14: 4:15
    }

    bb0: {
        _2 = discriminant(_1);           // bb0[0]: scope 0 at a.rs:3:9: 3:13
        switchInt(move _2) -> [0isize: bb3, 1isize: bb1, otherwise: bb2]; // bb0[1]: scope 0 at a.rs:3:9: 3:13
    }

    bb1: {
        StorageLive(_3);                 // bb1[0]: scope 0 at a.rs:4:14: 4:15
        _3 = move ((_1 as Some).0: std::boxed::Box<()>); // bb1[1]: scope 0 at a.rs:4:14: 4:15
        StorageLive(_4);                 // bb1[2]: scope 1 at a.rs:4:25: 4:26
        _4 = move _3;                    // bb1[3]: scope 1 at a.rs:4:25: 4:26
        ((_0 as Some).0: std::boxed::Box<()>) = move _4; // bb1[4]: scope 1 at a.rs:4:20: 4:27
        discriminant(_0) = 1;            // bb1[5]: scope 1 at a.rs:4:20: 4:27
        StorageDead(_4);                 // bb1[6]: scope 1 at a.rs:4:26: 4:27
        StorageDead(_3);                 // bb1[7]: scope 0 at a.rs:4:27: 4:28
        goto -> bb4;                     // bb1[8]: scope 0 at a.rs:2:5: 5:6
    }

    bb2: {
        unreachable;                     // bb2[0]: scope 0 at a.rs:2:11: 2:12
    }

    bb3: {
        discriminant(_0) = 0;            // bb3[0]: scope 0 at a.rs:3:17: 3:21
        goto -> bb4;                     // bb3[1]: scope 0 at a.rs:2:5: 5:6
    }

    bb4: {
        _5 = discriminant(_1);           // bb4[0]: scope 0 at a.rs:6:1: 6:2
        return;                          // bb4[1]: scope 0 at a.rs:6:2: 6:2
    }
}

The _5 = discriminant(_1); doesn't have any effect.

(Labeling A-cranelift because this prevents codegen of the goto -> bb4 like return. Which may result in a few extra instructions being emitted to move registers and jump to the return block for cg_clif.)

@rustbot modify labels: +A-mir +A-mir-opt +A-cranelift

@rustbot rustbot added A-cranelift Things relevant to the [future] cranelift backend A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt Area: MIR optimizations labels Mar 29, 2020
@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 29, 2020
@jonas-schievink
Copy link
Contributor

jonas-schievink commented Mar 29, 2020

We already have a pass that propagates the unreachable terminator, it should be possible to make it handle return as well.

(however return can leave sideeffects visible to the caller, so maybe this should be a proper dead code elimination pass instead)

@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented Mar 29, 2020

For background, this is a side-effect of #68528 (see the MIR diagram in the PR summary) and #68943. The MIR for the above example used to contain a frivolous open drop of x. After my changes to drop elaboration, all branches of the open drop become no-op Gotos, which are then removed by the SimplifyCfg pass. The read of the discriminant as part of the open drop remains, however.

To my knowledge, we don't have a dead-code elimination pass for the MIR, which would be the proper way to handle this.

@wesleywiser wesleywiser self-assigned this Mar 30, 2020
Centril added a commit to Centril/rust that referenced this issue Apr 2, 2020
…ant_reads, r=oli-obk

Remove unused discriminant reads from MIR bodies

Allow the `SimplifyLocals` pass to remove reads of discriminants if the
read is never used.

Fixes rust-lang#70531

r? @oli-obk
@bors bors closed this as completed in 4cba69e Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cranelift Things relevant to the [future] cranelift backend A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt Area: MIR optimizations C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants