Skip to content

Bitv indexing does not inline static values #19393

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

Closed
huonw opened this issue Nov 29, 2014 · 3 comments
Closed

Bitv indexing does not inline static values #19393

huonw opened this issue Nov 29, 2014 · 3 comments
Labels
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.

Comments

@huonw
Copy link
Member

huonw commented Nov 29, 2014

#![crate_type = "lib"]

pub fn foo(x: &std::collections::Bitv) -> bool {
    x[0]
}

With --opt-level=3:

_ZN3foo20h70da181d4ab1acbaeaaE:
    .cfi_startproc
    cmpq    %fs:112, %rsp
    ja  .LBB0_2
    movabsq $8, %r10
    movabsq $0, %r11
    callq   __morestack
    retq
.LBB0_2:
    pushq   %rax
.Ltmp0:
    .cfi_def_cfa_offset 16
    cmpq    $0, 24(%rdi)
    je  .LBB0_5
    cmpq    $0, 8(%rdi)
    je  .LBB0_6
    movq    (%rdi), %rax
    movq    _ZN3bit5FALSE20h4511a5a4abbc9859osaE@GOTPCREL(%rip), %rcx ; A
    testb   $1, (%rax)
    cmovneq _ZN3bit4TRUE20h4511a5a4abbc9859ksaE@GOTPCREL(%rip), %rcx
    movb    (%rcx), %al
    andb    $1, %al ; B
    popq    %rdx
    retq
.LBB0_5:
    movq    _ZN3bit4Bitv3get14_MSG_FILE_LINE20hefeb803ecea053eeLAaE@GOTPCREL(%rip), %rdi
    callq   _ZN9panicking5panic20hb082300345003b24XUlE@PLT
.LBB0_6:
    leaq    .Lconst2(%rip), %rdi
    xorl    %esi, %esi
    xorl    %edx, %edx
    callq   _ZN9panicking18panic_bounds_check20h46462e0c27f8d7d1tWlE@PLT
.Ltmp1:
    .size   _ZN3foo20h70da181d4ab1acbaeaaE, .Ltmp1-_ZN3foo20h70da181d4ab1acbaeaaE
    .cfi_endproc

In particular, there's the two ...mov...q _ZN3bit5...20h4511a5a4abbc9859osaE@GOTPCREL(%rip), %rcx lines, which are loading the addresses of TRUE and FALSE respectively. In the best case, the [] notation on a bitv will not do this since TRUE and FALSE truly are constants and are OK to inline (alternatively, it could just avoid doing the hack to use Index and use a by-value index instead).

@huonw huonw added the I-slow Issue: Problems and improvements with respect to performance of generated code. label Nov 29, 2014
@Aatch
Copy link
Contributor

Aatch commented Dec 19, 2014

Hmm, looks like the issue is that because indexing returns pointers to the global values LLVM can't inline them into user code. It doesn't actually know the real values. This can be confirmed by using lto, which gives LLVM access to the values.

@emberian emberian self-assigned this Mar 25, 2015
@emberian emberian removed their assignment Jan 5, 2016
@Mark-Simulacrum
Copy link
Member

std::collections::Bitv no longer exists, though I think the general issues noted here are still present. Is this just a case of LTO would let this happen, and without it, there's probably nothing we can do?

@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 22, 2017
@steveklabnik
Copy link
Member

With no response to @Mark-Simulacrum , I'm guessing this is true. Closing.

lnicola pushed a commit to lnicola/rust that referenced this issue Apr 28, 2025
Implicit field references during struct initialization were
being dropped because get_definition was returning None because
there were multiple definitions.

This adds a new helper, `get_defintions`, that supports returning
more than one definition for a given token and hooks it up.

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

No branches or pull requests

5 participants