Wrapper generates more instructions for simple integer operations

@rustbot

Code

I tried this code (godbolt):

#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub struct Int(u32);

const A: Int = Int(201);
const B: Int = Int(270);
const C: Int = Int(153);

#[inline(never)]
#[rustfmt::skip]
pub fn wrapped(x: Int) -> bool {
    (x >= A && x <= B)
        || x == C
}

#[inline(never)]
#[rustfmt::skip]
pub fn primitive(x: Int) -> bool {
    (x.0 >= A.0 && x.0 <= B.0)
        || x.0 == C.0
}

I expected to see this happen: both functions compile to identical assembly

Instead, this happened: wrapped contains way more instructions:

example::wrapped:
        cmp     edi, 201
        jb      .LBB0_3
        xor     eax, eax
        cmp     edi, 270
        mov     ecx, 255
        setne   al
        cmovae  ecx, eax
        mov     al, 1
        test    cl, cl
        je      .LBB0_4
        movzx   ecx, cl
        cmp     ecx, 255
        jne     .LBB0_3
.LBB0_4:
        ret
.LBB0_3:
        cmp     edi, 153
        sete    al
        ret

example::primitive:
        lea     eax, [rdi - 201]
        cmp     eax, 70
        setb    cl
        cmp     edi, 153
        sete    al
        or      al, cl
        ret

Version it worked on

It most recently worked on: 1.64

Version with regression

rustc --version --verbose:

rustc 1.77.0-nightly (e51e98dde 2023-12-31)
binary: rustc
commit-hash: e51e98dde6a60637b6a71b8105245b629ac3fe77
commit-date: 2023-12-31
host: x86_64-unknown-linux-gnu
release: 1.77.0-nightly
LLVM version: 17.0.6

I have bisected it to between 1.64 and 1.65 on Godbolt, and then further with cargo bisect-rustc which points to nightly-2022-08-17 (4033686...86c6ebe):

Regression in nightly-2022-08-17
...
looking for regression commit between 2022-08-16 and 2022-08-17
...
found 8 bors merge commits in the specified range
  commit[0] 2022-08-15: Auto merge of #100595 - matthiaskrgr:rollup-f1zur58, r=matthiaskrgr
  commit[1] 2022-08-15: Auto merge of #100007 - ChrisDenton:dtor-inline-never, r=michaelwoerister
  commit[2] 2022-08-16: Auto merge of #100237 - cjgillot:no-special-hash-hir, r=nagisa
  commit[3] 2022-08-16: Auto merge of #100611 - matthiaskrgr:rollup-rxj10ur, r=matthiaskrgr
  commit[4] 2022-08-16: Auto merge of #100441 - nnethercote:shrink-ast-Attribute, r=petrochenkov
  commit[5] 2022-08-16: Auto merge of #99612 - yanchen4791:issue-95079-fix, r=compiler-errors
  commit[6] 2022-08-16: Auto merge of #100626 - Dylan-DPC:rollup-mwbm7kj, r=Dylan-DPC
  commit[7] 2022-08-16: Auto merge of #100644 - TaKO8Ki:rollup-n0o6a1t, r=TaKO8Ki

Backtrace

<backtrace>

@rustbot modify labels: +I-slow +regression-from-stable-to-stable -regression-untriaged

@rustbot

@rustbot modify labels: -needs-triage +A-LLVM

Possibly #100460 which modified PartialOrd::le?

@rustbot

WG-prioritization assigning priority (Zulip discussion).

(I wonder if it's in any way related to issue #119014 also about code generation for Partial)

@rustbot label -I-prioritize +P-medium

Alive2: https://alive2.llvm.org/ce/z/LDK_Rf.
(I haven't checked where the missing optimizations are.)

@rustbot

@rustbot claim

Upstream issue: llvm/llvm-project#78281

Even though I created this issue, I think there may be other solutions.
I might be able to keep the range information in the SROA, and of course for this example, I could use the LazyValueInfo calculation. When I'm sure that this is also a possible way, I'll create a new issue. (Of course, the above issue needs to be resolved as well.)

But personally, I would like this issue to be solved directly in rust. I write similar code all the time (and I'm sure other projects are similar?). If we wait for an upstream solution, we may be delayed until LLVM 19. Of course, I would also like to see if I can use this to learn the MIR optimization. :)

Example of trait removal:

#![crate_type = "lib"]
use std::cmp::Ordering::*;

const A: i32 = 201;
const B: i32 = 270;
const C: i32 = 153;

#[inline(never)]
#[no_mangle]
pub fn foo(x: i32) -> bool {
    matches!(x.partial_cmp(&A), Some(Greater | Equal)) && 
    matches!(x.partial_cmp(&B), Some(Less | Equal)) ||
    x == C
}

Godbolt: https://rust.godbolt.org/z/EP9ohq5j3

Auto merge of rust-lang#120268 - DianQK:otherwise_is_last_variant_swi…

Rollup merge of rust-lang#120268 - DianQK:otherwise_is_last_variant_s…

Auto merge of rust-lang#120268 - DianQK:otherwise_is_last_variant_swi…

Navigation Menu

Repository navigation

Wrapper generates more instructions for simple integer operations #119520

Code

Version it worked on

Version with regression

Backtrace

1 remaining item

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Wrapper generates more instructions for simple integer operations #119520

Description

Code

Version it worked on

Version with regression

Backtrace

Activity

DaniPopes commented on Jan 2, 2024

DaniPopes commented on Jan 2, 2024

apiraino commented on Jan 4, 2024

dianqk commented on Jan 5, 2024

1 remaining item

dianqk commented on Jan 15, 2024

dianqk commented on Jan 16, 2024

dianqk commented on Jan 23, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions