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

Optimize matches!() invocations at the MIR level #75141

Closed
wesleywiser opened this issue Aug 4, 2020 · 4 comments
Closed

Optimize matches!() invocations at the MIR level #75141

wesleywiser opened this issue Aug 4, 2020 · 4 comments
Labels
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

@wesleywiser
Copy link
Member

Right now, matches!() generates MIR that sometimes has more branches than required. As an example:

fn foo(bar: Option<()>) {
    if matches!(bar, None) {
        stuff();
    }
}

generates this MIR:

fn foo(_1: std::option::Option<()>) -> () {
    debug bar => _1;
    let mut _0: ();
    let mut _2: bool;
    let mut _3: isize;
    let _4: ();

    bb0: {
        StorageLive(_2);
        _3 = discriminant(_1);
        switchInt(move _3) -> [0isize: bb2, otherwise: bb1];
    }

    bb1: {
        _2 = const false;
        goto -> bb3;
    }

    bb2: {
        _2 = const true;
        goto -> bb3;
    }

    bb3: {
        switchInt(_2) -> [false: bb4, otherwise: bb5];
    }

    bb4: {
        _0 = const ();
        goto -> bb7;
    }

    bb5: {
        StorageLive(_4);
        _4 = const stuff() -> bb6;
    }

    bb6: {
        StorageDead(_4);
        _0 = const ();
        goto -> bb7;
    }

    bb7: {
        StorageDead(_2);
        return;
    }
}

Notice blocks bb0, bb1, bb2 and bb3 which we could transform into something like this:

bb0: {
    StorageLive(_2);
    _3 = discriminant(_1);
    _2 = Eq(move _3, const 0isize);
    switchInt(_2) -> [false: bb4, otherwise: bb5];
}

There may be a compile-time win since we can hand LLVM less code but the optimization would have to be implemented to be sure.

cc @rust-lang/wg-mir-opt

@wesleywiser wesleywiser 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. A-mir-opt Area: MIR optimizations labels Aug 4, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Aug 4, 2020

Oooh, while wondering why you didn't suggest

bb0: {
    StorageLive(_2);
    _3 = discriminant(_1);
    switchInt(_3) -> [0: bb5, otherwise: bb4];
}

I realized that this is a common thing, too if you do if x == 42, so... #75144

@wesleywiser
Copy link
Member Author

Oh yeah, even better. I just trying to match the existing semantics as closely as possible.

bors added a commit to rust-lang-ci/rust that referenced this issue Aug 13, 2020
First iteration of simplify match branches

This is a simple MIR pass that attempts to convert
```
   bb0: {
        StorageLive(_2);
        _3 = discriminant(_1);
        switchInt(move _3) -> [0isize: bb2, otherwise: bb1];
    }

    bb1: {
        _2 = const false;
        goto -> bb3;
    }

    bb2: {
        _2 = const true;
        goto -> bb3;
    }
```
into
```
    bb0: {
        StorageLive(_2);
        _3 = discriminant(_1);
        _2 = _3 == 0;
        goto -> bb3;
    }
```
There are still missing components(like checking if the assignments are bools).
Was hoping that this could get some review though.

Handles rust-lang#75141

r? @oli-obk
@simonvandel
Copy link
Contributor

@wesleywiser This can be closed, fixed in #75382 , see https://godbolt.org/z/Pzv3vc

@wesleywiser
Copy link
Member Author

Thanks @simonvandel!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

No branches or pull requests

3 participants