Skip to content

Identity match on field-less enum compiles worse than using as #95313

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
scottmcm opened this issue Mar 25, 2022 · 2 comments
Closed

Identity match on field-less enum compiles worse than using as #95313

scottmcm opened this issue Mar 25, 2022 · 2 comments
Labels
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.

Comments

@scottmcm
Copy link
Member

Godbolt demo: https://rust.godbolt.org/z/Y35MWYxW8

Because Ordering has explicit discriminants, I expected these two functions to compile to the same thing:

use std::cmp::Ordering;

pub fn demo_match(order: Ordering) -> std::os::raw::c_int {
    match order {
        Ordering::Less => -1,
        Ordering::Equal => 0,
        Ordering::Greater => 1,
    }
}

pub fn demo_as(order: Ordering) -> std::os::raw::c_int {
    order as _
}

They don't, however:

example::demo_match:
        add     dil, 1
        movzx   eax, dil
        add     eax, -1
        ret

example::demo_as:
        movsx   eax, dil
        ret
define i32 @_ZN7example10demo_match17h0b9579071593c5f7E(i8 noundef %0) unnamed_addr #0 !dbg !5 {
start:
  %switch.tableidx = add i8 %0, 1, !dbg !10
  %switch.idx.cast = zext i8 %switch.tableidx to i32, !dbg !10
  %switch.offset = add nsw i32 %switch.idx.cast, -1, !dbg !10
  ret i32 %switch.offset, !dbg !11
}

define i32 @_ZN7example7demo_as17h328bc074f9126c77E(i8 noundef %order) unnamed_addr #0 !dbg !12 {
start:
  %0 = sext i8 %order to i32, !dbg !13
  ret i32 %0, !dbg !14
}

Noticed while replying to https://users.rust-lang.org/t/std-ordering-as-integer/73450/5?u=scottmcm

@scottmcm scottmcm 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. labels Mar 25, 2022
@scottmcm
Copy link
Member Author

scottmcm commented Mar 25, 2022

Alive confirmed that LLVM could just do this, so I've also filed llvm/llvm-project#54561

But I'll leave this open too because clang seems to optimize the equivalent switch fine (https://clang.godbolt.org/z/565Wcf8fK), so it's possible that in Rust we should be doing something better.

EDIT: oh, clang sexts before the switch. Here's a clang repro (using different types) https://clang.godbolt.org/z/57jqWxbxe, so I'll close this issue for now -- it can come back as E-needs-test when the upstream issue is fixed.

@inclyc
Copy link

inclyc commented Feb 11, 2023

Opened https://reviews.llvm.org/D143720

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
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.
Projects
None yet
Development

No branches or pull requests

2 participants