Skip to content

Optimization regression in 1.32+ #59352

Open
@adrian17

Description

@adrian17

The following function:

pub fn num_to_digit(num: char) -> u32 {
    if num.is_digit(8) { num.to_digit(8).unwrap() } else { 0 }
}

With 1.31 produced the following:

  add edi, -48
  xor eax, eax
  cmp edi, 8
  cmovb eax, edi
  ret

Which looks reasonable, as unwrap() should never fail.
But in 1.32 and above it produces an unnecessary panic branch:

  push rax
  mov ecx, edi
  and ecx, -8
  xor eax, eax
  cmp ecx, 48
  jne .LBB0_3
  add edi, -48
  mov eax, edi
  cmp edi, 8
  jae .LBB0_2
.LBB0_3:
  pop rcx
  ret
.LBB0_2:
  lea rdi, [rip + .L__unnamed_1]
  call qword ptr [rip + _ZN4core9panicking5panic17h6f50c0de2dcd7974E@GOTPCREL]
  ud2

Interestingly, when I manually inline is_digit:

#[inline]
pub fn is_digit(self, radix: u32) -> bool {
self.to_digit(radix).is_some()
}

pub fn num_to_digit_fast1(num: char) -> u32 {
    if num.to_digit(8).is_some() { num.to_digit(8).unwrap() } else { 0 }
}

Or just use unwrap_or:

pub fn num_to_digit_fast2(num: char) -> u32 {
    num.to_digit(8).unwrap_or(0)
}

It produces good code on all compiler versions.

Godbolt instance I tested this on: https://godbolt.org/z/8yg7x0

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-codegenArea: Code generationC-optimizationCategory: An issue highlighting optimization opportunities or PRs implementing suchE-needs-testCall for participation: An issue has been fixed and does not reproduce, but no test has been added.I-slowIssue: Problems and improvements with respect to performance of generated code.P-mediumMedium priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions