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

miri ICE promote const: impossible case reached #53201

Closed
gnzlbg opened this issue Aug 8, 2018 · 10 comments · Fixed by #54816
Closed

miri ICE promote const: impossible case reached #53201

gnzlbg opened this issue Aug 8, 2018 · 10 comments · Fixed by #54816
Assignees
Labels
A-const-eval Area: constant evaluation (mir interpretation) A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 8, 2018

Playground:

#![feature(const_fn, rustc_attrs, attr_literals, untagged_unions)] 

/// A single unconstrained byte (0-255).
#[derive(Copy, Clone, Debug)]
pub struct ImmByte(u8);
impl ImmByte {
    /// Constructor
    #[inline]
    #[rustc_args_required_const(0)]
    pub const fn new(value: u8) -> Self {
        ImmByte(value)
    }
}

pub struct v128([u8; 16]);

impl v128 {
    #[inline]
    #[rustc_args_required_const(0)]
    pub const unsafe fn const_(imm: [ImmByte; 16]) -> v128 {
        union U {
            imm: [ImmByte; 16],
            vec: v128,
        }
        U { imm }.vec
    }
}

fn main() {
    let _a = v128::const_([
        ImmByte::new(0),
        ImmByte::new(1),
        ImmByte::new(2),
        ImmByte::new(3),
        ImmByte::new(4),
        ImmByte::new(5),
        ImmByte::new(6),
        ImmByte::new(7),
        ImmByte::new(8),
        ImmByte::new(9),
        ImmByte::new(10),
        ImmByte::new(11),
        ImmByte::new(12),
        ImmByte::new(13),
        ImmByte::new(14),
        ImmByte::new(15),
    ]);
}

produces

error: internal compiler error: librustc_mir/transform/promote_consts.rs:334: impossible case reached

thread 'main' panicked at 'Box<Any>', librustc_errors/lib.rs:578:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: aborting due to previous error

cc @oli-obk

@oli-obk oli-obk added A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. A-const-eval Area: constant evaluation (mir interpretation) labels Aug 8, 2018
@cramertj
Copy link
Member

cramertj commented Aug 9, 2018

Not that this fixes the issue, but [ImmByte; 16] and v128([u8; 16]) aren't necessarily guaranteed to have the same representation without #[repr(C)] or #[repr(transparent)] on both v128 and ImmByte.

@oli-obk
Copy link
Contributor

oli-obk commented Aug 9, 2018

minified:

#![feature(const_fn, rustc_attrs, attr_literals)] 

#[rustc_args_required_const(0)]
pub const fn new(value: u8) -> u8 {
    value
}

#[rustc_args_required_const(0)]
pub unsafe fn const_(_: u8) {
    unimplemented!()
}

fn main() {
    let _a = const_(new(0));
}

https://play.rust-lang.org/?gist=2032873a12df78fc3e4537d26735a050&version=nightly&mode=debug&edition=2015

@eddyb is this a victim of the backwards algorithm for promotion? Just a hunch, but it might have promoted away the call to new and then end up trying to promote the 0, but it's not in the place where it was expected, because it now lives inside the promoted. So it ends up trying to promote an argument in a basic block that was already promoted away

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Aug 9, 2018

thanks @cramertj , I'll update the PR that triggered this - for context rust-lang/stdarch#549

@oli-obk
Copy link
Contributor

oli-obk commented Aug 9, 2018

As a workaround until we fix this ICE, you can introduce an intermediate explicit constant for the argument to v128::const_. Or is this part of a user-exposed API?

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Aug 9, 2018

This is part of a user-exposed API, but that workaround should allow me to at least get the const tests running.

@eddyb
Copy link
Member

eddyb commented Aug 18, 2018

Just a hunch, but it might have promoted away the call to new and then end up trying to promote the 0, but it's not in the place where it was expected, because it now lives inside the promoted.

It won't remove the definition if there's more than one use (unless this is fallout from your changes?)

@oli-obk
Copy link
Contributor

oli-obk commented Aug 18, 2018

There's only one use. I don't see how my changes could have caused this, but it seems possible. I'll do a bisection

@oli-obk oli-obk self-assigned this Aug 18, 2018
@eddyb
Copy link
Member

eddyb commented Aug 18, 2018

Oh, I see, #[rustc_args_required_const] is Candidate::Argument and that's missing a check like:

if temps[local] == TempState::PromotedOut {
// Already promoted.
continue;
}

@oli-obk oli-obk added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Aug 27, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Aug 27, 2018

Fix instructions:

  • Add a IndexVec<BasicBlock, bool> which, like the temps variable, is used to mark blocks whose terminator has been promoted out.
  • skip promoting all such terminators when encountering said block id in a Candidate::Argument (
    Candidate::Argument { .. } => {}
    )

@eddyb
Copy link
Member

eddyb commented Sep 8, 2018

@oli-obk You can use a BitVector<BasicBool>, although I'm not even sure it's necessary.
A promoted call ends up as a Goto, so you could just skip it based on that?

kennytm added a commit to kennytm/rust that referenced this issue Oct 26, 2018
Don't try to promote already promoted out temporaries

fixes rust-lang#53201

r? @eddyb
kennytm added a commit to kennytm/rust that referenced this issue Oct 26, 2018
Don't try to promote already promoted out temporaries

fixes rust-lang#53201

r? @eddyb
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 (mir interpretation) A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants