Avoid loop_match self-assignment in MIR lowering by cijiugechu · Pull Request #155186 · rust-lang/rust

cijiugechu · 2026-04-12T06:38:38Z

Transform

bb2: {
        PlaceMention(_1);
        _1 = copy _1;
        goto -> bb7;
    }

to

bb2: {
        PlaceMention(_1);
        _4 = copy _1;
        _1 = copy _4;
        goto -> bb7;
    }

Closes #143806

Previous MIR

fn helper() -> u8 {
    let mut _0: u8;
    let mut _1: u8;
    let mut _2: !;
    let mut _3: !;
    scope 1 {
        debug state => _1;
    }

    bb0: {
        StorageLive(_1);
        _1 = const 0_u8;
        FakeRead(ForLet(None), _1);
        StorageLive(_2);
        goto -> bb1;
    }

    bb1: {
        falseUnwind -> [real: bb2, unwind: bb11];
    }

    bb2: {
        PlaceMention(_1);
        _1 = copy _1;
        goto -> bb7;
    }

    bb3: {
        FakeRead(ForMatchedPlace(None), _1);
        unreachable;
    }

    bb4: {
        unreachable;
    }

    bb5: {
        goto -> bb6;
    }

    bb6: {
        goto -> bb8;
    }

    bb7: {
        goto -> bb8;
    }

    bb8: {
        goto -> bb1;
    }

    bb9: {
        unreachable;
    }

    bb10: {
        StorageDead(_2);
        StorageDead(_1);
        return;
    }

    bb11 (cleanup): {
        resume;
    }
}

Current MIR

fn helper() -> u8 {
    let mut _0: u8;
    let mut _1: u8;
    let mut _2: !;
    let mut _3: !;
    let mut _4: u8;
    scope 1 {
        debug state => _1;
    }

    bb0: {
        StorageLive(_1);
        _1 = const 0_u8;
        FakeRead(ForLet(None), _1);
        StorageLive(_2);
        goto -> bb1;
    }

    bb1: {
        falseUnwind -> [real: bb2, unwind: bb11];
    }

    bb2: {
        PlaceMention(_1);
        _4 = copy _1;
        _1 = copy _4;
        goto -> bb7;
    }

    bb3: {
        FakeRead(ForMatchedPlace(None), _1);
        unreachable;
    }

    bb4: {
        unreachable;
    }

    bb5: {
        goto -> bb6;
    }

    bb6: {
        goto -> bb8;
    }

    bb7: {
        goto -> bb8;
    }

    bb8: {
        goto -> bb1;
    }

    bb9: {
        unreachable;
    }

    bb10: {
        StorageDead(_2);
        StorageDead(_1);
        return;
    }

    bb11 (cleanup): {
        resume;
    }
}

rustbot · 2026-04-12T06:38:43Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

rustbot · 2026-04-12T06:38:45Z

r? @jdonszelmann

rustbot has assigned @jdonszelmann.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 69 candidates
Random selection from 11 candidates

folkertdev · 2026-04-12T14:54:13Z

Just conceptually, would it not be better to remove the assignment completely? Or does that not work?

I've previously tried to change the code so that the assignment does not get introduced, but that did not work out (it would, from memory, require overhauling a whole bunch of code).

cijiugechu · 2026-04-12T15:50:01Z

Just conceptually, would it not be better to remove the assignment completely? Or does that not work?

I've previously tried to change the code so that the assignment does not get introduced, but that did not work out (it would, from memory, require overhauling a whole bunch of code).

I did consider this approach at first, but I was worried that it might have a fairly significant impact on MIR semantics. Once Rvalue::Use(Operand::Copy(_1)) is removed, it could also affect borrowck’s analysis, so in the end I chose the more conservative approach.

cijiugechu · 2026-04-12T15:53:41Z

Additionally, I also considered the performance impact. I compared the LLVM IR generated by the compiler for loop_match before and after the change, and they were the same, so there should not bring performance regression.

jdonszelmann · 2026-04-16T20:43:54Z

r? @folkertdev

folkertdev · 2026-04-20T13:31:34Z

Just conceptually, would it not be better to remove the assignment completely? Or does that not work?
I've previously tried to change the code so that the assignment does not get introduced, but that did not work out (it would, from memory, require overhauling a whole bunch of code).

I did consider this approach at first, but I was worried that it might have a fairly significant impact on MIR semantics. Once Rvalue::Use(Operand::Copy(_1)) is removed, it could also affect borrowck’s analysis, so in the end I chose the more conservative approach.

I'm sceptical of this if the original MIR had a Copy operation, but I'm not really sure.

r? WaffleLapkin do you know, or else who should we ask about this?

At a high level the fix does make sense to me, and the tests look good, so r=me for that.

rustbot · 2026-04-20T13:31:40Z

WaffleLapkin is not on the review rotation at the moment.
They may take a while to respond.

rustbot · 2026-04-26T06:39:15Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

folkertdev

This seems fine to me, unless Waffle has lingering concerns. The copy should just be cleaned up by later MIR passes, or in any case by LLVM.

Linking for posterity #t-compiler/mir-opts > Semantic of assignment.

View changes since this review

WaffleLapkin · 2026-05-01T10:48:48Z

@bors r=folkertdev,WaffleLapkin

rust-bors · 2026-05-01T10:48:51Z

📌 Commit 52bd93d has been approved by folkertdev,WaffleLapkin

It is now in the queue for this repository.

…uwer Rollup of 9 pull requests Successful merges: - #149637 (Do not run jump-threading for GPUs) - #154971 (Verify that penultimate segment of enum variant path refers to enum if it has args) - #155186 (Avoid loop_match self-assignment in MIR lowering) - #155948 (Fix order-dependent visibility diagnostics) - #156001 (ssa-range-prop: fix ICE when encountering self-domiating bb) - #155600 (Adds a couple UI tests for polonius) - #155995 (-Zembed-source: also embed external source) - #156019 (Feed cleanups) - #156031 (Return a single diagnostic from `lex_token_trees`.)

Rollup merge of #155186 - cijiugechu:fix/loop-match-no-self-assign, r=folkertdev,WaffleLapkin Avoid loop_match self-assignment in MIR lowering Transform ``` bb2: { PlaceMention(_1); _1 = copy _1; goto -> bb7; } ``` to ``` bb2: { PlaceMention(_1); _4 = copy _1; _1 = copy _4; goto -> bb7; } ``` Closes #143806 <details> <summary>Previous MIR</summary> ``` fn helper() -> u8 { let mut _0: u8; let mut _1: u8; let mut _2: !; let mut _3: !; scope 1 { debug state => _1; } bb0: { StorageLive(_1); _1 = const 0_u8; FakeRead(ForLet(None), _1); StorageLive(_2); goto -> bb1; } bb1: { falseUnwind -> [real: bb2, unwind: bb11]; } bb2: { PlaceMention(_1); _1 = copy _1; goto -> bb7; } bb3: { FakeRead(ForMatchedPlace(None), _1); unreachable; } bb4: { unreachable; } bb5: { goto -> bb6; } bb6: { goto -> bb8; } bb7: { goto -> bb8; } bb8: { goto -> bb1; } bb9: { unreachable; } bb10: { StorageDead(_2); StorageDead(_1); return; } bb11 (cleanup): { resume; } } ``` </details> <details> <summary>Current MIR</summary> ``` fn helper() -> u8 { let mut _0: u8; let mut _1: u8; let mut _2: !; let mut _3: !; let mut _4: u8; scope 1 { debug state => _1; } bb0: { StorageLive(_1); _1 = const 0_u8; FakeRead(ForLet(None), _1); StorageLive(_2); goto -> bb1; } bb1: { falseUnwind -> [real: bb2, unwind: bb11]; } bb2: { PlaceMention(_1); _4 = copy _1; _1 = copy _4; goto -> bb7; } bb3: { FakeRead(ForMatchedPlace(None), _1); unreachable; } bb4: { unreachable; } bb5: { goto -> bb6; } bb6: { goto -> bb8; } bb7: { goto -> bb8; } bb8: { goto -> bb1; } bb9: { unreachable; } bb10: { StorageDead(_2); StorageDead(_1); return; } bb11 (cleanup): { resume; } } ``` </details>

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 12, 2026

rustbot assigned jdonszelmann Apr 12, 2026

folkertdev added the F-loop_match when you match up with someone and they really throw you for a loop label Apr 12, 2026

rustbot assigned folkertdev and unassigned jdonszelmann Apr 16, 2026

rustbot assigned WaffleLapkin and unassigned folkertdev Apr 20, 2026

WaffleLapkin reviewed Apr 24, 2026

View reviewed changes

Comment thread tests/mir-opt/building/loop_match_no_self_assign.rs

WaffleLapkin reviewed Apr 24, 2026

View reviewed changes

Comment thread compiler/rustc_mir_build/src/builder/expr/into.rs Outdated

WaffleLapkin added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 24, 2026

This comment has been minimized.

Sign in to view

cijiugechu added 3 commits April 26, 2026 14:24

Avoid loop_match self-assignment in MIR lowering

c5f8952

Move logic to ExprKind::LoopMatch

446d34c

Fix old directive

52bd93d

cijiugechu force-pushed the fix/loop-match-no-self-assign branch from e347227 to 52bd93d Compare April 26, 2026 06:39

cijiugechu added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 26, 2026

folkertdev approved these changes Apr 27, 2026

View reviewed changes

WaffleLapkin approved these changes May 1, 2026

View reviewed changes

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 1, 2026

JonathanBrouwer mentioned this pull request May 1, 2026

Rollup of 9 pull requests #156041

Merged

rust-bors Bot merged commit 9996d99 into rust-lang:main May 1, 2026
11 checks passed

rustbot added this to the 1.97.0 milestone May 1, 2026

Uh oh!

Conversation

cijiugechu commented Apr 12, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 12, 2026

Uh oh!

rustbot commented Apr 12, 2026

Uh oh!

folkertdev commented Apr 12, 2026

Uh oh!

cijiugechu commented Apr 12, 2026

Uh oh!

cijiugechu commented Apr 12, 2026

Uh oh!

jdonszelmann commented Apr 16, 2026

Uh oh!

folkertdev commented Apr 20, 2026

Uh oh!

rustbot commented Apr 20, 2026

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

rustbot commented Apr 26, 2026

Uh oh!

folkertdev left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WaffleLapkin commented May 1, 2026

Uh oh!

rust-bors Bot commented May 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

cijiugechu commented Apr 12, 2026 •

edited by rustbot

Loading

folkertdev left a comment •

edited by rustbot

Loading