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

Remove length check from ThinSlice::clone to halve generated code #76

Merged
merged 2 commits into from
Nov 29, 2023

Conversation

arthurprs
Copy link
Contributor

@arthurprs arthurprs commented Nov 29, 2023

Remove length check from ThinSlice::clone. This check doubles the size of the function generated code. Since ThinSlice::clone is inlined everywhere this can improve codegen significantly.

Includes a commit bumping the version to 0.1.10.

Asm before

        .cfi_startproc
        sub rsp, 56
        .cfi_def_cfa_offset 64

        mov rax, qword ptr [rdi]

        mov rcx, qword ptr [rax + 8]

        lock inc        qword ptr [rax]

        jle .LBB366_3

        mov qword ptr [rsp], rcx

        cmp qword ptr [rax + 8], rcx

        jne .LBB366_4

        add rsp, 56
        .cfi_def_cfa_offset 8
        ret

.LBB366_3:
        .cfi_def_cfa_offset 64
        call qword ptr [rip + std::process::abort@GOTPCREL]

        ud2

.LBB366_4:
        lea rcx, [rip + .L__unnamed_289]

        mov qword ptr [rsp + 8], rcx
        mov qword ptr [rsp + 16], 1
        mov qword ptr [rsp + 24], 0
        lea rcx, [rip + .L__unnamed_183]
        mov qword ptr [rsp + 40], rcx
        mov qword ptr [rsp + 48], 0
        add rax, 8

        mov rsi, rsp
        lea rdx, [rsp + 8]
        mov rdi, rax

        call core::panicking::assert_failed
        ud2

@arthurprs arthurprs changed the title Remove length check from ThinSlice::clone Remove length check from ThinSlice::clone to halve generated code Nov 29, 2023
@Manishearth Manishearth merged commit d3654b8 into Manishearth:master Nov 29, 2023
4 checks passed
@Manishearth
Copy link
Owner

Thanks!

@Manishearth
Copy link
Owner

Manishearth commented Nov 29, 2023

Ugh, oops, I made a mistake here, the 0.1.10 bump was already on my local master. I'm going to force push to fix this and repush your commit

@Manishearth
Copy link
Owner

Pushed 4b219e3

@Manishearth
Copy link
Owner

Pushed 4b219e3. Make a separate PR for 0.1.11 if you need it.

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