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

Promoted constants lead to poor MIR from comparisons #139093

Open
scottmcm opened this issue Mar 29, 2025 · 0 comments
Open

Promoted constants lead to poor MIR from comparisons #139093

scottmcm opened this issue Mar 29, 2025 · 0 comments
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt Area: MIR optimizations needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-mir-opt Working group: MIR optimizations

Comments

@scottmcm
Copy link
Member

Follow-along at https://rust.godbolt.org/z/Trzrobvo5

Consider these two functions that ought to be equivalent:

use std::cmp::Ordering;

#[no_mangle]
pub fn direct(e: Option<Ordering>) -> bool {
    e == Some(Ordering::Equal)
}

#[no_mangle]
pub fn with_let(e: Option<Ordering>) -> bool {
    let eq = Ordering::Equal;
    e == Some(eq)
}

The latter ends up with about the MIR you'd expect: it checks for Some, then for Equal

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

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

    bb2: {
        StorageLive(_3);
        _3 = discriminant(((_1 as Some).0: std::cmp::Ordering));
        _0 = Eq(copy _3, const 0_i8);
        StorageDead(_3);
        goto -> bb3;
    }

    bb3: {
        StorageDead(_2);
        return;
    }

    bb4: {
        unreachable;
    }

The former, though, ends up with way more MIR because of the promoted const:

    bb0: {
        _2 = const direct::promoted[0];
        StorageLive(_5);
        StorageLive(_4);
        StorageLive(_3);
        _3 = discriminant(_1);
        switchInt(move _3) -> [0: bb1, 1: bb2, otherwise: bb6];
    }

    bb1: {
        _4 = discriminant((*_2));
        _0 = Eq(copy _4, const 0_isize);
        goto -> bb5;
    }

    bb2: {
        _5 = discriminant((*_2));
        switchInt(move _5) -> [0: bb3, 1: bb4, otherwise: bb6];
    }

    bb3: {
        _0 = const false;
        goto -> bb5;
    }

    bb4: {
        StorageLive(_6);
        StorageLive(_7);
        _6 = discriminant(((_1 as Some).0: std::cmp::Ordering));
        _7 = discriminant((((*_2) as Some).0: std::cmp::Ordering));
        _0 = Eq(copy _6, copy _7);
        StorageDead(_7);
        StorageDead(_6);
        goto -> bb5;
    }

    bb5: {
        StorageDead(_3);
        StorageDead(_4);
        StorageDead(_5);
        return;
    }

    bb6: {
        unreachable;
    }

Apparently we can't even tell what the discriminant is of that const direct::promoted[0], so we end up emitting even LLVM IR checks on the value of the constant, which it then has to turn around and optimize away again.

Can we maybe just unilaterally un-promote simple (Copy + NoCell + Scalar/ScalarPair) constants like this?

@scottmcm scottmcm added A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt Area: MIR optimizations T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-mir-opt Working group: MIR optimizations labels Mar 29, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt Area: MIR optimizations needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-mir-opt Working group: MIR optimizations
Projects
None yet
Development

No branches or pull requests

2 participants