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

inline iter in repr/primitive.rs #48

Merged
merged 1 commit into from
Oct 11, 2023
Merged

Conversation

Fuuzetsu
Copy link
Contributor

We have a piece of simple code that looks a bit like this:

   let action = self.iter().next()?;
   self.0.remove(action);
   Some(action)

where self.iter() just defers to EnumSet::iter().

Looking at the asm with cargo asm, I see something like below. There's a non-inlined call to iter. After adding #[inline] in enumset, you can see at the bottom snipped that the call disappears and assembly ends up being much better.

.section .text.trader::policy::ToDoList::pop,"ax",@progbits
        .globl  trader::policy::ToDoList::pop
        .p2align        4, 0x90
        .type   trader::policy::ToDoList::pop,@function
trader::policy::ToDoList::pop:

                // /home/shana/programming/engine/trader/src/policy.rs : 309
                pub fn pop(&mut self) -> Option<TraderAction> {
        .cfi_startproc
        push rbp
        .cfi_def_cfa_offset 16
        push rbx
        .cfi_def_cfa_offset 24
        push rax
        .cfi_def_cfa_offset 32
        .cfi_offset rbx, -24
        .cfi_offset rbp, -16
        mov rbx, rdi

                // /home/shana/programming/engine/trader/src/policy.rs : 329
                self.0.iter()
        mov ebp, dword ptr [rdi]

                // /home/shana/.cargo/registry/src/index.crates.io-6f17d22bba15001f/enumset-1.1.2/src/set.rs : 760
                EnumSetIter { iter: set.__priv_repr.iter() }
        mov edi, ebp

        call qword ptr [rip + enumset::repr::primitive::_::<impl enumset::repr::EnumSetTypeRepr for u32>::iter@GOTPCREL]
        mov ecx, eax
        mov al, 21

                // /home/shana/.cargo/registry/src/index.crates.io-6f17d22bba15001f/enumset-1.1.2/src/repr/primitive.rs : 20
                *self == 0
        test ecx, ecx

                // /home/shana/.cargo/registry/src/index.crates.io-6f17d22bba15001f/enumset-1.1.2/src/repr/primitive.rs : 275
                if self.0.is_empty() {
        je .LBB4674_4

                // /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/num/mod.rs : 1143
                uint_impl! {
        rep bsf ecx, ecx

                // /home/shana/programming/engine/trader/src/action.rs : 36
                enumset::EnumSetType,
        cmp ecx, 21
        jb .LBB4674_3

        cmp cl, 21
        jne .LBB4674_3

.LBB4674_4:
                // /home/shana/programming/engine/trader/src/policy.rs : 313
                }
        add rsp, 8
        .cfi_def_cfa_offset 24
        pop rbx

        .cfi_def_cfa_offset 16
        pop rbp
        .cfi_def_cfa_offset 8
        ret

.LBB4674_3:
        .cfi_def_cfa_offset 32
                // /home/shana/.cargo/registry/src/index.crates.io-6f17d22bba15001f/enumset-1.1.2/src/repr/primitive.rs : 29
                *self &= !(1 << bit as $name);
        btr ebp, ecx
        mov dword ptr [rbx], ebp
        mov eax, ecx

                // /home/shana/programming/engine/trader/src/policy.rs : 313
                }
        add rsp, 8
        .cfi_def_cfa_offset 24
        pop rbx

        .cfi_def_cfa_offset 16
        pop rbp
        .cfi_def_cfa_offset 8
        ret
.section .text.trader::policy::ToDoList::pop,"ax",@progbits
        .globl  trader::policy::ToDoList::pop
        .p2align        4, 0x90
        .type   trader::policy::ToDoList::pop,@function
trader::policy::ToDoList::pop:

        .cfi_startproc
                // /home/shana/programming/engine/trader/src/policy.rs : 329
                self.0.iter()
        mov ecx, dword ptr [rdi]
        mov al, 21

                // /tmp/enumset/enumset/src/repr/primitive.rs : 20
                *self == 0
        test ecx, ecx

                // /tmp/enumset/enumset/src/repr/primitive.rs : 276
                if self.0.is_empty() {
        je .LBB4674_4

                // /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/num/uint_macros.rs : 170
                intrinsics::cttz(self) as u32
        rep bsf edx, ecx

                // /home/shana/programming/engine/trader/src/action.rs : 36
                enumset::EnumSetType,
        cmp edx, 21
        jb .LBB4674_3

        cmp dl, 21
        jne .LBB4674_3

.LBB4674_4:
                // /home/shana/programming/engine/trader/src/policy.rs : 313
                }
        ret

.LBB4674_3:
                // /tmp/enumset/enumset/src/repr/primitive.rs : 29
                *self &= !(1 << bit as $name);
        btr ecx, edx
        mov dword ptr [rdi], ecx
        mov eax, edx

                // /home/shana/programming/engine/trader/src/policy.rs : 313
                }
        ret

We have a piece of simple code that looks a bit like this:

```rust
   let action = self.iter().next()?;
   self.0.remove(action);
   Some(action)
```

where `self.iter()` just defers to `EnumSet::iter()`.

Looking at the asm with `cargo asm`, I see something like below. There's
a non-inlined call to `iter`. After adding `#[inline]` in `enumset`, you
can see at the bottom snipped that the call disappears and assembly ends
up being much better.

```asm
.section .text.trader::policy::ToDoList::pop,"ax",@progbits
        .globl  trader::policy::ToDoList::pop
        .p2align        4, 0x90
        .type   trader::policy::ToDoList::pop,@function
trader::policy::ToDoList::pop:

                // /home/shana/programming/engine/trader/src/policy.rs : 309
                pub fn pop(&mut self) -> Option<TraderAction> {
        .cfi_startproc
        push rbp
        .cfi_def_cfa_offset 16
        push rbx
        .cfi_def_cfa_offset 24
        push rax
        .cfi_def_cfa_offset 32
        .cfi_offset rbx, -24
        .cfi_offset rbp, -16
        mov rbx, rdi

                // /home/shana/programming/engine/trader/src/policy.rs : 329
                self.0.iter()
        mov ebp, dword ptr [rdi]

                // /home/shana/.cargo/registry/src/index.crates.io-6f17d22bba15001f/enumset-1.1.2/src/set.rs : 760
                EnumSetIter { iter: set.__priv_repr.iter() }
        mov edi, ebp

        call qword ptr [rip + enumset::repr::primitive::_::<impl enumset::repr::EnumSetTypeRepr for u32>::iter@GOTPCREL]
        mov ecx, eax
        mov al, 21

                // /home/shana/.cargo/registry/src/index.crates.io-6f17d22bba15001f/enumset-1.1.2/src/repr/primitive.rs : 20
                *self == 0
        test ecx, ecx

                // /home/shana/.cargo/registry/src/index.crates.io-6f17d22bba15001f/enumset-1.1.2/src/repr/primitive.rs : 275
                if self.0.is_empty() {
        je .LBB4674_4

                // /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/num/mod.rs : 1143
                uint_impl! {
        rep bsf ecx, ecx

                // /home/shana/programming/engine/trader/src/action.rs : 36
                enumset::EnumSetType,
        cmp ecx, 21
        jb .LBB4674_3

        cmp cl, 21
        jne .LBB4674_3

.LBB4674_4:
                // /home/shana/programming/engine/trader/src/policy.rs : 313
                }
        add rsp, 8
        .cfi_def_cfa_offset 24
        pop rbx

        .cfi_def_cfa_offset 16
        pop rbp
        .cfi_def_cfa_offset 8
        ret

.LBB4674_3:
        .cfi_def_cfa_offset 32
                // /home/shana/.cargo/registry/src/index.crates.io-6f17d22bba15001f/enumset-1.1.2/src/repr/primitive.rs : 29
                *self &= !(1 << bit as $name);
        btr ebp, ecx
        mov dword ptr [rbx], ebp
        mov eax, ecx

                // /home/shana/programming/engine/trader/src/policy.rs : 313
                }
        add rsp, 8
        .cfi_def_cfa_offset 24
        pop rbx

        .cfi_def_cfa_offset 16
        pop rbp
        .cfi_def_cfa_offset 8
        ret
```

```asm
.section .text.trader::policy::ToDoList::pop,"ax",@progbits
        .globl  trader::policy::ToDoList::pop
        .p2align        4, 0x90
        .type   trader::policy::ToDoList::pop,@function
trader::policy::ToDoList::pop:

        .cfi_startproc
                // /home/shana/programming/engine/trader/src/policy.rs : 329
                self.0.iter()
        mov ecx, dword ptr [rdi]
        mov al, 21

                // /tmp/enumset/enumset/src/repr/primitive.rs : 20
                *self == 0
        test ecx, ecx

                // /tmp/enumset/enumset/src/repr/primitive.rs : 276
                if self.0.is_empty() {
        je .LBB4674_4

                // /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/num/uint_macros.rs : 170
                intrinsics::cttz(self) as u32
        rep bsf edx, ecx

                // /home/shana/programming/engine/trader/src/action.rs : 36
                enumset::EnumSetType,
        cmp edx, 21
        jb .LBB4674_3

        cmp dl, 21
        jne .LBB4674_3

.LBB4674_4:
                // /home/shana/programming/engine/trader/src/policy.rs : 313
                }
        ret

.LBB4674_3:
                // /tmp/enumset/enumset/src/repr/primitive.rs : 29
                *self &= !(1 << bit as $name);
        btr ecx, edx
        mov dword ptr [rdi], ecx
        mov eax, edx

                // /home/shana/programming/engine/trader/src/policy.rs : 313
                }
        ret
```
@Lymia Lymia merged commit b61c9b3 into Lymia:main Oct 11, 2023
21 checks passed
@Fuuzetsu Fuuzetsu deleted the inline-repr-prim branch October 12, 2023 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants