refactor!: split_first_chunk to help optimizer remove unreachable panic#65
Merged
Noratrieb merged 2 commits intorust-lang:masterfrom Mar 25, 2026
Merged
Conversation
I'm not interested in performance, actually. The reason I need the optimizer to do well here is to eliminate a panic condition. I'm trying to write a no-panic hash table and tried using rustc-hash and discovered this.
Member
|
Thanks, bumping the MSRV to that old version is no problem. I'm gonna review it later. cc @orlp you might also be interested |
morrisonlevi
commented
Mar 25, 2026
orlp
approved these changes
Mar 25, 2026
Contributor
orlp
left a comment
orlp
left a comment
There was a problem hiding this comment.
Seems fine to me, I just don't understand why the old code got pessimized.
Noratrieb
approved these changes
Mar 25, 2026
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This does two things:
!in the title. The MSRV bump is forsplit_first_chunk.hash_bytespath to remove a panic branch.I'm trying to write a no-panic hash table and tried using rustc-hash and discovered this. The difficulty of writing no-panic code is that it's dependent on the optimizer. The v1.77 of Rust doesn't have a panic in the generated code whereas Rust v1.92 does (aarch64-apple-darwin). The code on this branch optimizes cleanly for both Rust versions.
Here you can see the diff in assembly and see that the
blwithslice_index_failis gone on the right (and is just generally nicer):aarch64-apple-darwn side-by-side assembly
movk x13, #10655, lsl #16 | ldp x8, x14, [x13], #16 movk x13, #14370, lsl #32 | eor x8, x8, x9 movk x13, #41993, lsl #48 | eor x9, x14, x11 LBB0_4: | mul x14, x9, x8 mov x14, x9 < add x9, x11, #8 < cmp x9, x2 < b.hi LBB0_13 < add x11, x11, #16 < ldp x9, x15, [x12, #-8] < eor x8, x9, x8 < eor x9, x15, x13 < mul x15, x9, x8 < umulh x8, x9, x8 umulh x8, x9, x8 eor x9, x8, x15 | eor x8, x8, x14 add x12, x12, #16 | mov x9, x12 mov x8, x14 | cmp x10, #15 cmp x11, x10 | b.hi LBB0_5 b.lo LBB0_4 | add x9, x0, x1 add x8, x0, x10 | ldp x10, x11, [x9, #-16] ldp x10, x11, [x8] | eor x9, x10, x12 eor x8, x10, x14 | eor x8, x11, x8 eor x9, x11, x9 | mul x10, x9, x8 b LBB0_12 | umulh x8, x9, x8 > eor x8, x8, x10 > eor x0, x1, x8 > ret LBB0_7: LBB0_7: cmp x2, #3 | cmp x1, #3 b.ls LBB0_9 b.ls LBB0_9 ldr w10, [x0] ldr w10, [x0] add x11, x0, x2 | add x11, x0, x1 ldur w11, [x11, #-4] ldur w11, [x11, #-4] eor x8, x10, x8 | eor x9, x10, x9 eor x9, x11, x9 | eor x8, x11, x8 b LBB0_12 | mul x10, x9, x8 > umulh x8, x9, x8 > eor x8, x8, x10 > eor x0, x1, x8 > ret LBB0_9: LBB0_9: cbz x2, LBB0_12 | cbz x1, LBB0_3 lsr x10, x2, #1 | lsr x10, x1, #1 ldrb w10, [x0, x10] ldrb w10, [x0, x10] ldrb w11, [x0] ldrb w11, [x0] add x12, x0, x2 | add x12, x0, x1 ldurb w12, [x12, #-1] ldurb w12, [x12, #-1] eor x8, x11, x8 | eor x9, x11, x9 orr x10, x10, x12, lsl #8 orr x10, x10, x12, lsl #8 LBB0_11: | eor x8, x10, x8 eor x9, x10, x9 | mul x10, x9, x8 LBB0_12: | umulh x8, x9, x8 mul x10, x8, x9 < umulh x8, x8, x9 < eor x8, x8, x10 eor x8, x8, x10 eor x0, x2, x8 | eor x0, x1, x8 .cfi_def_cfa wsp, 16 < ldp x29, x30, [sp], #16 < .cfi_def_cfa_offset 0 < .cfi_restore w30 < .cfi_restore w29 < ret ret LBB0_13: < .cfi_restore_state < add x8, x2, #8 < and x0, x8, #0xfffffffffffffff0 < Ltmp0: < Lloh0: < adrp x3, l_anon.e881f07e3afd45838d38e6d12340fccf.1 < Lloh1: < add x3, x3, l_anon.e881f07e3afd45838d38e6d12340fc < orr x1, x0, #0x8 < bl __ZN4core5slice5index16slice_index_fail17h548 < Ltmp1: < brk #0x1 < LBB0_15: < Ltmp2: < bl __ZN4core9panicking19panic_cannot_unwind17he0 < .loh AdrpAdd Lloh0, Lloh1 < Lfunc_end0: < .cfi_endproc .cfi_endproc .section __TEXT,__gcc_except_tab < .p2align 2, 0x0 < GCC_except_table0: < Lexception0: < .byte 255 < .byte 155 < .uleb128 Lttbase0-Lttbaseref0 < Lttbaseref0: < .byte 1 < .uleb128 Lcst_end0-Lcst_begin0 < Lcst_begin0: < .uleb128 Ltmp0-Lfunc_begin0 < .uleb128 Ltmp1-Ltmp0 < .uleb128 Ltmp2-Lfunc_begin0 < .byte 1 < Lcst_end0: < .byte 127 < .byte 0 < .p2align 2, 0x0 < Lttbase0: < .byte 0 < .p2align 2, 0x0 < < .section __TEXT,__cstring,cstring_literals < l_anon.e881f07e3afd45838d38e6d12340fccf.0: < .asciz "src/lib.rs" < < .section __DATA,__const < .p2align 3, 0x0 < l_anon.e881f07e3afd45838d38e6d12340fccf.1: < .quad l_anon.e881f07e3afd45838d38e6d12340fccf.0 < .asciz "\n\000\000\000\000\000\000\0009\001\000\000- < .subsections_via_symbols .subsections_via_symbolsThis can be solved in other ways that do not require an MSRV bump, but I could not personally find one that worked that avoided
unsafe, which seems to also be a goal here.I used an
extern "C"stub like this to be able to see the assembly: