MIR building: Place should be moved if not used later #75993

simonvandel · 2020-08-27T18:19:40Z

The following MIR can be improved by converting a copy to a move: https://godbolt.org/z/jnKK4f .
Observed

fn opt_char(_1: char) -> u32 {
    debug x => _1;                       // in scope 0 at ./example.rs:1:17: 1:18
    let mut _0: u32;                     // return place in scope 0 at ./example.rs:1:29: 1:32
    let mut _2: bool;                    // in scope 0 at ./example.rs:2:8: 2:28
    let mut _3: char;                    // in scope 0 at ./example.rs:2:8: 2:9
    let mut _4: bool;                    // in scope 0 at ./example.rs:2:20: 2:28
    let mut _5: char;                    // in scope 0 at ./example.rs:2:20: 2:21

    bb0: {
        StorageLive(_2);                 // scope 0 at ./example.rs:2:8: 2:28
        StorageLive(_3);                 // scope 0 at ./example.rs:2:8: 2:9
        _3 = _1;                         // scope 0 at ./example.rs:2:8: 2:9
        switchInt(move _3) -> ['b': bb1, otherwise: bb2]; // scope 0 at ./example.rs:2:8: 2:28
    }

    bb1: {
        StorageDead(_3);                 // scope 0 at ./example.rs:2:8: 2:28
        _2 = const true;                 // scope 0 at ./example.rs:2:8: 2:28
        goto -> bb3;                     // scope 0 at ./example.rs:2:8: 2:28
    }

    bb2: {
        StorageDead(_3);                 // scope 0 at ./example.rs:2:8: 2:28
        StorageLive(_4);                 // scope 0 at ./example.rs:2:20: 2:28
        StorageLive(_5);                 // scope 0 at ./example.rs:2:20: 2:21
        _5 = _1;                         // scope 0 at ./example.rs:2:20: 2:21
        _4 = Eq(move _5, const 'a');     // scope 0 at ./example.rs:2:20: 2:28
        StorageDead(_5);                 // scope 0 at ./example.rs:2:27: 2:28
        _2 = Ne(_4, const false);        // scope 0 at ./example.rs:2:8: 2:28
        goto -> bb3;                     // scope 0 at ./example.rs:2:8: 2:28
    }

    bb3: {
        StorageDead(_4);                 // scope 0 at ./example.rs:2:27: 2:28
        switchInt(_2) -> [false: bb4, otherwise: bb5]; // scope 0 at ./example.rs:2:5: 2:45
    }

    bb4: {
        _0 = const 1_u32;                // scope 0 at ./example.rs:2:42: 2:43
        goto -> bb6;                     // scope 0 at ./example.rs:2:5: 2:45
    }

    bb5: {
        _0 = const 0_u32;                // scope 0 at ./example.rs:2:31: 2:32
        goto -> bb6;                     // scope 0 at ./example.rs:2:5: 2:45
    }

    bb6: {
        StorageDead(_2);                 // scope 0 at ./example.rs:3:1: 3:2
        return;                          // scope 0 at ./example.rs:3:2: 3:2
    }
}

Expected: _2 is moved into the switchInt as it has no uses later. _4 could similarly be moved into the Ne comparison

fn opt_char(_1: char) -> u32 {
    debug x => _1;                       // in scope 0 at ./example.rs:1:17: 1:18
    let mut _0: u32;                     // return place in scope 0 at ./example.rs:1:29: 1:32
    let mut _2: bool;                    // in scope 0 at ./example.rs:2:8: 2:28
    let mut _3: char;                    // in scope 0 at ./example.rs:2:8: 2:9
    let mut _4: bool;                    // in scope 0 at ./example.rs:2:20: 2:28
    let mut _5: char;                    // in scope 0 at ./example.rs:2:20: 2:21

    bb0: {
        StorageLive(_2);                 // scope 0 at ./example.rs:2:8: 2:28
        StorageLive(_3);                 // scope 0 at ./example.rs:2:8: 2:9
        _3 = _1;                         // scope 0 at ./example.rs:2:8: 2:9
        switchInt(move _3) -> ['b': bb1, otherwise: bb2]; // scope 0 at ./example.rs:2:8: 2:28
    }

    bb1: {
        StorageDead(_3);                 // scope 0 at ./example.rs:2:8: 2:28
        _2 = const true;                 // scope 0 at ./example.rs:2:8: 2:28
        goto -> bb3;                     // scope 0 at ./example.rs:2:8: 2:28
    }

    bb2: {
        StorageDead(_3);                 // scope 0 at ./example.rs:2:8: 2:28
        StorageLive(_4);                 // scope 0 at ./example.rs:2:20: 2:28
        StorageLive(_5);                 // scope 0 at ./example.rs:2:20: 2:21
        _5 = _1;                         // scope 0 at ./example.rs:2:20: 2:21
        _4 = Eq(move _5, const 'a');     // scope 0 at ./example.rs:2:20: 2:28
        StorageDead(_5);                 // scope 0 at ./example.rs:2:27: 2:28
        _2 = Ne(move _4, const false);        // scope 0 at ./example.rs:2:8: 2:28
        goto -> bb3;                     // scope 0 at ./example.rs:2:8: 2:28
    }

    bb3: {
        StorageDead(_4);                 // scope 0 at ./example.rs:2:27: 2:28
        switchInt(move _2) -> [false: bb4, otherwise: bb5]; // scope 0 at ./example.rs:2:5: 2:45
    }

    bb4: {
        _0 = const 1_u32;                // scope 0 at ./example.rs:2:42: 2:43
        goto -> bb6;                     // scope 0 at ./example.rs:2:5: 2:45
    }

    bb5: {
        _0 = const 0_u32;                // scope 0 at ./example.rs:2:31: 2:32
        goto -> bb6;                     // scope 0 at ./example.rs:2:5: 2:45
    }

    bb6: {
        StorageDead(_2);                 // scope 0 at ./example.rs:3:1: 3:2
        return;                          // scope 0 at ./example.rs:3:2: 3:2
    }
}

Original issue - no longer relevant

In the following snippet https://godbolt.org/z/6f3xs3 , I would have expected _2 to be moved into the switchInt since it is not used afterwards.

This was noticed in #75370 (comment) where the proposed MIR-optimization currently fails to remove a comparison..

Observed:

fn f(_1: i8) -> i8 {
    debug x => _1;                       // in scope 0 at ./example.rs:1:6: 1:7
    let mut _0: i8;                      // return place in scope 0 at ./example.rs:1:16: 1:18
    let mut _2: bool;                    // in scope 0 at ./example.rs:2:8: 2:15

    bb0: {
        StorageLive(_2);                 // scope 0 at ./example.rs:2:8: 2:15
        _2 = Eq(move _1, const 42_i8);   // scope 0 at ./example.rs:2:8: 2:15
        switchInt(_2) -> [false: bb1, otherwise: bb2]; // scope 0 at ./example.rs:2:5: 2:32
    }

    bb1: {
        _0 = const 1_i8;                 // scope 0 at ./example.rs:2:29: 2:30
        goto -> bb3;                     // scope 0 at ./example.rs:2:5: 2:32
    }

    bb2: {
        _0 = const 0_i8;                 // scope 0 at ./example.rs:2:18: 2:19
        goto -> bb3;                     // scope 0 at ./example.rs:2:5: 2:32
    }

    bb3: {
        StorageDead(_2);                 // scope 0 at ./example.rs:3:1: 3:2
        return;                          // scope 0 at ./example.rs:3:2: 3:2
    }
}

Expected:

fn f(_1: i8) -> i8 {
    debug x => _1;                       // in scope 0 at ./example.rs:1:6: 1:7
    let mut _0: i8;                      // return place in scope 0 at ./example.rs:1:16: 1:18
    let mut _2: bool;                    // in scope 0 at ./example.rs:2:8: 2:15

    bb0: {
        StorageLive(_2);                 // scope 0 at ./example.rs:2:8: 2:15
        _2 = Eq(move _1, const 42_i8);   // scope 0 at ./example.rs:2:8: 2:15
        switchInt(move _2) -> [false: bb1, otherwise: bb2]; // scope 0 at ./example.rs:2:5: 2:32
    }

    bb1: {
        _0 = const 1_i8;                 // scope 0 at ./example.rs:2:29: 2:30
        goto -> bb3;                     // scope 0 at ./example.rs:2:5: 2:32
    }

    bb2: {
        _0 = const 0_i8;                 // scope 0 at ./example.rs:2:18: 2:19
        goto -> bb3;                     // scope 0 at ./example.rs:2:5: 2:32
    }

    bb3: {
        StorageDead(_2);                 // scope 0 at ./example.rs:3:1: 3:2
        return;                          // scope 0 at ./example.rs:3:2: 3:2
    }
}

The text was updated successfully, but these errors were encountered:

jonas-schievink · 2020-08-27T18:21:41Z

We don't have a pass that promotes copies to moves, but that's something that could probably be implemented.

jumbatm · 2020-09-03T03:59:10Z

pass that promotes copies to moves

Hi, I'm interested in implementing this. @simonvandel -- I know this issue came up on your PR. Were you intending on also taking this?

simonvandel · 2020-09-03T05:34:06Z

Were you intending on also taking this?

Go ahead 🙂

jumbatm · 2020-09-03T05:35:13Z

Great, thanks!

@rustbot claim

jumbatm · 2020-09-05T12:16:40Z

Hmm, it looks like SimplifyComparisonIntegral already converts that copy to a move for the example you gave:

Before:

// MIR for `f` before SimplifyComparisonIntegral

fn f(_1: i8) -> i8 {
...
    bb0: {
        StorageLive(_2);                 // scope 0 at switchInt.rs:4:9: 4:10
        _2 = _1;                         // scope 0 at switchInt.rs:4:13: 4:14
        StorageLive(_3);                 // scope 1 at switchInt.rs:5:8: 5:15
        StorageLive(_4);                 // scope 1 at switchInt.rs:5:8: 5:9
        _4 = _2;                         // scope 1 at switchInt.rs:5:8: 5:9
        _3 = Eq(move _4, const 42_i8);   // scope 1 at switchInt.rs:5:8: 5:15
        StorageDead(_4);                 // scope 1 at switchInt.rs:5:14: 5:15
        switchInt(_3) -> [false: bb1, otherwise: bb2]; // scope 1 at switchInt.rs:5:5: 5:32
    }
...
}

After:

// MIR for `f` after SimplifyComparisonIntegral                                                                                                     
                                                                                                                                                    
fn f(_1: i8) -> i8 {                                                                                                                                
...
    bb0: {
        StorageLive(_2);                 // scope 0 at switchInt.rs:4:9: 4:10
        _2 = _1;                         // scope 0 at switchInt.rs:4:13: 4:14
        StorageLive(_3);                 // scope 1 at switchInt.rs:5:8: 5:15
        StorageLive(_4);                 // scope 1 at switchInt.rs:5:8: 5:9
        _4 = _2;                         // scope 1 at switchInt.rs:5:8: 5:9
        _3 = Eq(_4, const 42_i8);        // scope 1 at switchInt.rs:5:8: 5:15
        nop;                             // scope 1 at switchInt.rs:5:14: 5:15
        switchInt(move _4) -> [42_i8: bb2, otherwise: bb1]; // scope 1 at switchInt.rs:5:5: 5:32
    }
...
    bb1: {
        StorageDead(_4);                 // scope 1 at switchInt.rs:5:5: 5:32
        ...
    }
    bb2: {
        StorageDead(_4);                 // scope 1 at switchInt.rs:5:5: 5:32
        ...
   }
...
}

I'm now trying to understand this comment here: #75370 (comment) to see whether I can chip in. In the code you linked, you mention that your pass generates code that moves _3 twice, but I only see a single move _3. Are you instead referring to that you generate two StorageDead(_3) statements, the same thing happens for _4 here?

simonvandel · 2020-09-06T09:31:55Z

Huh, in the godbolt link in the issue description (https://godbolt.org/z/6f3xs3) the MIR is optimized as I would expect. So that is perhaps not the best example anymore. I do however still think that the following MIR can be improved by converting a copy to a move: https://godbolt.org/z/jnKK4f . I'll update the issue description with the new example.
Observed

fn opt_char(_1: char) -> u32 {
    debug x => _1;                       // in scope 0 at ./example.rs:1:17: 1:18
    let mut _0: u32;                     // return place in scope 0 at ./example.rs:1:29: 1:32
    let mut _2: bool;                    // in scope 0 at ./example.rs:2:8: 2:28
    let mut _3: char;                    // in scope 0 at ./example.rs:2:8: 2:9
    let mut _4: bool;                    // in scope 0 at ./example.rs:2:20: 2:28
    let mut _5: char;                    // in scope 0 at ./example.rs:2:20: 2:21

    bb0: {
        StorageLive(_2);                 // scope 0 at ./example.rs:2:8: 2:28
        StorageLive(_3);                 // scope 0 at ./example.rs:2:8: 2:9
        _3 = _1;                         // scope 0 at ./example.rs:2:8: 2:9
        switchInt(move _3) -> ['b': bb1, otherwise: bb2]; // scope 0 at ./example.rs:2:8: 2:28
    }

    bb1: {
        StorageDead(_3);                 // scope 0 at ./example.rs:2:8: 2:28
        _2 = const true;                 // scope 0 at ./example.rs:2:8: 2:28
        goto -> bb3;                     // scope 0 at ./example.rs:2:8: 2:28
    }

    bb2: {
        StorageDead(_3);                 // scope 0 at ./example.rs:2:8: 2:28
        StorageLive(_4);                 // scope 0 at ./example.rs:2:20: 2:28
        StorageLive(_5);                 // scope 0 at ./example.rs:2:20: 2:21
        _5 = _1;                         // scope 0 at ./example.rs:2:20: 2:21
        _4 = Eq(move _5, const 'a');     // scope 0 at ./example.rs:2:20: 2:28
        StorageDead(_5);                 // scope 0 at ./example.rs:2:27: 2:28
        _2 = Ne(_4, const false);        // scope 0 at ./example.rs:2:8: 2:28
        goto -> bb3;                     // scope 0 at ./example.rs:2:8: 2:28
    }

    bb3: {
        StorageDead(_4);                 // scope 0 at ./example.rs:2:27: 2:28
        switchInt(_2) -> [false: bb4, otherwise: bb5]; // scope 0 at ./example.rs:2:5: 2:45
    }

    bb4: {
        _0 = const 1_u32;                // scope 0 at ./example.rs:2:42: 2:43
        goto -> bb6;                     // scope 0 at ./example.rs:2:5: 2:45
    }

    bb5: {
        _0 = const 0_u32;                // scope 0 at ./example.rs:2:31: 2:32
        goto -> bb6;                     // scope 0 at ./example.rs:2:5: 2:45
    }

    bb6: {
        StorageDead(_2);                 // scope 0 at ./example.rs:3:1: 3:2
        return;                          // scope 0 at ./example.rs:3:2: 3:2
    }
}

Expected: _2 is moved into the switchInt as it has no uses later. _4 could similarly be moved into the Ne comparison

fn opt_char(_1: char) -> u32 {
    debug x => _1;                       // in scope 0 at ./example.rs:1:17: 1:18
    let mut _0: u32;                     // return place in scope 0 at ./example.rs:1:29: 1:32
    let mut _2: bool;                    // in scope 0 at ./example.rs:2:8: 2:28
    let mut _3: char;                    // in scope 0 at ./example.rs:2:8: 2:9
    let mut _4: bool;                    // in scope 0 at ./example.rs:2:20: 2:28
    let mut _5: char;                    // in scope 0 at ./example.rs:2:20: 2:21

    bb0: {
        StorageLive(_2);                 // scope 0 at ./example.rs:2:8: 2:28
        StorageLive(_3);                 // scope 0 at ./example.rs:2:8: 2:9
        _3 = _1;                         // scope 0 at ./example.rs:2:8: 2:9
        switchInt(move _3) -> ['b': bb1, otherwise: bb2]; // scope 0 at ./example.rs:2:8: 2:28
    }

    bb1: {
        StorageDead(_3);                 // scope 0 at ./example.rs:2:8: 2:28
        _2 = const true;                 // scope 0 at ./example.rs:2:8: 2:28
        goto -> bb3;                     // scope 0 at ./example.rs:2:8: 2:28
    }

    bb2: {
        StorageDead(_3);                 // scope 0 at ./example.rs:2:8: 2:28
        StorageLive(_4);                 // scope 0 at ./example.rs:2:20: 2:28
        StorageLive(_5);                 // scope 0 at ./example.rs:2:20: 2:21
        _5 = _1;                         // scope 0 at ./example.rs:2:20: 2:21
        _4 = Eq(move _5, const 'a');     // scope 0 at ./example.rs:2:20: 2:28
        StorageDead(_5);                 // scope 0 at ./example.rs:2:27: 2:28
        _2 = Ne(move _4, const false);        // scope 0 at ./example.rs:2:8: 2:28
        goto -> bb3;                     // scope 0 at ./example.rs:2:8: 2:28
    }

    bb3: {
        StorageDead(_4);                 // scope 0 at ./example.rs:2:27: 2:28
        switchInt(move _2) -> [false: bb4, otherwise: bb5]; // scope 0 at ./example.rs:2:5: 2:45
    }

    bb4: {
        _0 = const 1_u32;                // scope 0 at ./example.rs:2:42: 2:43
        goto -> bb6;                     // scope 0 at ./example.rs:2:5: 2:45
    }

    bb5: {
        _0 = const 0_u32;                // scope 0 at ./example.rs:2:31: 2:32
        goto -> bb6;                     // scope 0 at ./example.rs:2:5: 2:45
    }

    bb6: {
        StorageDead(_2);                 // scope 0 at ./example.rs:3:1: 3:2
        return;                          // scope 0 at ./example.rs:3:2: 3:2
    }
}

jumbatm · 2021-03-14T00:46:42Z

Sorry for the delay. Unassigning myself because I can't find a trivial case which would benefit from this pass (ie, we seem to already be producing code with the appropriate moves; the above code included).

If anyone finds another case which would benefit from this pass, I'm happy to pick this up again (and should be able to get to it faster now that I've got less on my plate).

simonvandel mentioned this issue Aug 27, 2020

New pass to optimize ifconditions on integrals to switches on the integer #75370

Merged

jonas-schievink added 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. labels Aug 27, 2020

rustbot assigned jumbatm Sep 3, 2020

jumbatm mentioned this issue Sep 8, 2020

Incorrect MIR optimizations at level 3: local incorrectly marked dead #76432

Closed

jumbatm removed their assignment Mar 14, 2021

cjgillot mentioned this issue Jul 19, 2023

Turn copy into moves during DSE. #113758

Merged

bors closed this as completed in 06a53dd Jul 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MIR building: Place should be moved if not used later #75993

MIR building: Place should be moved if not used later #75993

simonvandel commented Aug 27, 2020 •

edited

Loading

jonas-schievink commented Aug 27, 2020

jumbatm commented Sep 3, 2020

simonvandel commented Sep 3, 2020

jumbatm commented Sep 3, 2020

jumbatm commented Sep 5, 2020

simonvandel commented Sep 6, 2020 •

edited

Loading

jumbatm commented Mar 14, 2021 •

edited

Loading

MIR building: Place should be moved if not used later #75993

MIR building: Place should be moved if not used later #75993

Comments

simonvandel commented Aug 27, 2020 • edited Loading

jonas-schievink commented Aug 27, 2020

jumbatm commented Sep 3, 2020

simonvandel commented Sep 3, 2020

jumbatm commented Sep 3, 2020

jumbatm commented Sep 5, 2020

simonvandel commented Sep 6, 2020 • edited Loading

jumbatm commented Mar 14, 2021 • edited Loading

simonvandel commented Aug 27, 2020 •

edited

Loading

simonvandel commented Sep 6, 2020 •

edited

Loading

jumbatm commented Mar 14, 2021 •

edited

Loading