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

rustc fails to remove dead panic! code when unwrapping an Option #48253

Closed
glandium opened this issue Feb 16, 2018 · 2 comments
Closed

rustc fails to remove dead panic! code when unwrapping an Option #48253

glandium opened this issue Feb 16, 2018 · 2 comments
Labels
A-codegen Area: Code generation C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@glandium
Copy link
Contributor

Consider the following code:

pub enum NodeColor {
    Black = 0,
    Red = 1,
}

impl NodeColor {
    fn from_usize_fast(i: usize) -> Self {
        match i {
            0 => NodeColor::Black,
            1 => NodeColor::Red,
            _ => panic!(),
        }
    }   

    fn from_usize(i: usize) -> Option<Self> {
        match i {
            0 => Some(NodeColor::Black),
            1 => Some(NodeColor::Red),
            _ => None,
        }
    }   
}   

pub fn to_color_fast(c: usize) -> NodeColor {
    NodeColor::from_usize_fast(c & 1)
}

pub fn to_color(c: usize) -> NodeColor {
    NodeColor::from_usize(c & 1).unwrap()
}

The & 1 ensures the panicking conditions can never happen. This is the corresponding code generated on godbolt with 1.24 beta and 1.25 nightly:

example::to_color_fast:
  push rbp
  mov rbp, rsp
  and edi, 1
  mov eax, edi
  pop rbp
  ret

example::to_color:
  mov rcx, rdi
  and rcx, 1
  and rdi, 1
  je .LBB1_1
  mov al, 2
  sub al, cl
  cmp al, 2
  jne .LBB1_5
  jmp .LBB1_4
.LBB1_1:
  xor eax, eax
  cmp al, 2
  je .LBB1_4
.LBB1_5:
  and al, 1
  ret
.LBB1_4:
  push rbp
  mov rbp, rsp
  lea rdi, [rip + .Lref.2]
  call core::panicking::panic@PLT
  ud2

str.0:
  .ascii "called `Option::unwrap()` on a `None` value"

str.1:
  .ascii "/checkout/src/libcore/option.rs"

.Lref.2:
  .quad str.0
  .quad 43
  .quad str.1
  .quad 31
  .long 335
  .long 21

Note how to_color goes through hoops. But the most notable thing is that this is actually a regression from 1.24, because versions up to an including 1.23 were actually generating the same code for both functions.

@Centril Centril added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-slow Issue: Problems and improvements with respect to performance of generated code. A-codegen Area: Code generation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. labels Feb 16, 2018
@jkordish jkordish added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Apr 18, 2018
@nikic
Copy link
Contributor

nikic commented Dec 18, 2018

Codegen for both functions is identical since 1.29. Unless there's some other case where this issue still occurs, I'd consider this fixed.

@LingMan
Copy link
Contributor

LingMan commented Apr 12, 2020

As of 1.42.0 the given example compiles down to:

playground::to_color_fast:
	movq	%rdi, %rax
	andb	$1, %al
	retq
.set playground::to_color, playground::to_color_fast

Looks fixed to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. 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

6 participants