MIR building: Place should be moved if not used later

[simonvandel]

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
    }
}

[jonas-schievink]

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

[jumbatm]

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]

Were you intending on also taking this?

Go ahead 🙂

[jumbatm]

Great, thanks!

@rustbot claim

[jumbatm]

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]

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]

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).