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

Compile-time regression with mir-opt-level=2 #73717

Closed
RalfJung opened this issue Jun 25, 2020 · 2 comments · Fixed by #77373
Closed

Compile-time regression with mir-opt-level=2 #73717

RalfJung opened this issue Jun 25, 2020 · 2 comments · Fixed by #77373
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code. requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Jun 25, 2020

The following rustc floating point test case used to work just fine with -Zmir-opt-level=3, but now even just calling rustc -Zmir-opt-level=2 on it diverges:

#![feature(track_caller, stmt_expr_attributes)]

// Poor-man's black-box
#[inline(never)]
fn black_box<T>(x: T) -> T { x }

macro_rules! test {
    ($val:expr, $src_ty:ident -> $dest_ty:ident, $expected:expr) => (
        // black_box disables constant evaluation to test run-time conversions:
        assert_eq!(black_box::<$src_ty>($val) as $dest_ty, $expected,
                    "run-time {} -> {}", stringify!($src_ty), stringify!($dest_ty));

        {
            const X: $src_ty = $val;
            const Y: $dest_ty = X as $dest_ty;
            assert_eq!(Y, $expected,
                        "const eval {} -> {}", stringify!($src_ty), stringify!($dest_ty));
        }
    );

    ($fval:expr, f* -> $ity:ident, $ival:expr) => (
        test!($fval, f32 -> $ity, $ival);
        test!($fval, f64 -> $ity, $ival);
    )
}

macro_rules! common_fptoi_tests {
    ($fty:ident -> $($ity:ident)+) => ({ $(
        test!($fty::NAN, $fty -> $ity, 0);
        test!($fty::INFINITY, $fty -> $ity, $ity::MAX);
        test!($fty::NEG_INFINITY, $fty -> $ity, $ity::MIN);
        // These two tests are not solely float->int tests, in particular the latter relies on
        // `u128::MAX as f32` not being UB. But that's okay, since this file tests int->float
        // as well, the test is just slightly misplaced.
        test!($ity::MIN as $fty, $fty -> $ity, $ity::MIN);
        test!($ity::MAX as $fty, $fty -> $ity, $ity::MAX);
        test!(0., $fty -> $ity, 0);
        test!($fty::MIN_POSITIVE, $fty -> $ity, 0);
        test!(-0.9, $fty -> $ity, 0);
        test!(1., $fty -> $ity, 1);
        test!(42., $fty -> $ity, 42);
    )+ });

    (f* -> $($ity:ident)+) => ({
        common_fptoi_tests!(f32 -> $($ity)+);
        common_fptoi_tests!(f64 -> $($ity)+);
    })
}

macro_rules! fptoui_tests {
    ($fty: ident -> $($ity: ident)+) => ({ $(
        test!(-0., $fty -> $ity, 0);
        test!(-$fty::MIN_POSITIVE, $fty -> $ity, 0);
        test!(-0.99999994, $fty -> $ity, 0);
        test!(-1., $fty -> $ity, 0);
        test!(-100., $fty -> $ity, 0);
        test!(#[allow(overflowing_literals)] -1e50, $fty -> $ity, 0);
        test!(#[allow(overflowing_literals)] -1e130, $fty -> $ity, 0);
    )+ });

    (f* -> $($ity:ident)+) => ({
        fptoui_tests!(f32 -> $($ity)+);
        fptoui_tests!(f64 -> $($ity)+);
    })
}

fn main() {
    common_fptoi_tests!(f* -> i8 i16 i32 i64 u8 u16 u32 u64);
    fptoui_tests!(f* -> u8 u16 u32 u64);
    common_fptoi_tests!(f* -> i128 u128);
    fptoui_tests!(f* -> u128);
}

Found through the Miri test suite. Unfortunately we have barely any test coverage on the rustc side for mir-opt-level 2 and higher.

Cc @rust-lang/wg-mir-opt

@RalfJung RalfJung added the requires-nightly This issue requires a nightly compiler in some way. label Jun 25, 2020
@jonas-schievink jonas-schievink added A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 25, 2020
@spastorino spastorino added I-prioritize Issue: Indicates that prioritization has been requested for this issue. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jun 25, 2020
@spastorino
Copy link
Member

I was too fast to request prioritization but then realized that this is requires-nightly. Anyway we've briefly discussed this on Zulip.

@RalfJung
Copy link
Member Author

Ah I should also add the regression range: 033013c was still good. 67100f6 has the bug.

Is it appropriate to summon the ICE breakers for this?

@RalfJung RalfJung added the I-slow Issue: Problems and improvements with respect to performance of generated code. label Sep 7, 2020
@bors bors closed this as completed in ffeeb20 Oct 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code. requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
3 participants