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

Missed optimization opportunity when trivially moving struct by moving fields #135786

Open
recatek opened this issue Jan 20, 2025 · 0 comments
Open
Labels
C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@recatek
Copy link

recatek commented Jan 20, 2025

Example code:

#[repr(C)]
pub struct ThreeFields<'a> {
    a: &'a [u32],
    b: &'a [u32],
    c: &'a [u32],
}

#[inline(always)]
pub fn should_be_no_op(val: ThreeFields) -> ThreeFields {
    ThreeFields {
        a: val.a,
        b: val.b,
        c: val.c,
    }
}

pub fn sum_slices_1(val: ThreeFields) -> u32 {
    sum(&val)
}

pub fn sum_slices_2(val: ThreeFields) -> u32 {
    let val = should_be_no_op(val);
    sum(&val)
}

#[inline(never)]
pub fn sum(val: &ThreeFields) -> u32 {
    val.a.iter().sum::<u32>() + val.b.iter().sum::<u32>() + val.c.iter().sum::<u32>()
}

In rustc 1.84 stable this generates a number of moves that I don't think need to be there, especially when inlining:

example::sum_slices_1::hc2a0527df8a4985d:
        jmp     qword ptr [rip + example::sum::h228167780f08fdbb@GOTPCREL]

example::sum_slices_2::hfa5e09ed5de6d084:
        sub     rsp, 56
        movups  xmm0, xmmword ptr [rdi]
        movups  xmm1, xmmword ptr [rdi + 16]
        movups  xmm2, xmmword ptr [rdi + 32]
        movups  xmmword ptr [rsp + 8], xmm0
        movups  xmmword ptr [rsp + 24], xmm1
        movups  xmmword ptr [rsp + 40], xmm2
        lea     rdi, [rsp + 8]
        call    qword ptr [rip + example::sum::h228167780f08fdbb@GOTPCREL]
        add     rsp, 56
        ret

See https://rust.godbolt.org/z/erxfM53Kq

While this is a pretty pointless example, this comes up in situations where you might want to convert a tuple of slices into a struct of slices in order to assign names to the tuple members (I've been running into these issues working on gecs).

See also: #107436 and #135787

@recatek recatek added the C-bug Category: This is a bug. label Jan 20, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 20, 2025
@recatek recatek changed the title Missed opportunity when trivially moving struct by moving fields. Missed optimization opportunity when trivially moving struct by moving fields. Jan 20, 2025
@recatek recatek changed the title Missed optimization opportunity when trivially moving struct by moving fields. Missed optimization opportunity when trivially moving struct by moving fields Jan 20, 2025
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such 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

3 participants