Manual vs compiler bounds checking codegen

[jgarvin]

I wanted to see what the bounds checking generated code looked like, and whether I could get the same assembly when writing my own check. This is what I tried:

// ignore the unbound lifetime, only paying attention to codegen
pub fn manual_bounds_check<'a>(p: *mut u8, l: usize, range: std::ops::Range<usize>) -> &'a mut [u8] {
    assert!(range.start < l && range.end <= l && range.start <= range.end);
    unsafe {
        let start = p.add(range.start);
        &mut *std::ptr::slice_from_raw_parts_mut(start, range.len())
    }
}

pub fn compiler_bounds_check(p: &mut [u8], range: std::ops::Range<usize>) -> &mut [u8] {
    &mut p[range]
}

Compiler Explorer link

Results are mixed:

1. Manual does three separate comparisons (worse)
2. Manual only generates the code for panicking once (better at least for perf, but means the error you get is less specific)

For #1, is there a way to get better codegen? For #2 maybe the compiler could be tweaked to call a common failure function for both cases that would inspect the registers to figure out which panic message to use? Otherwise every slice[a..b] generates ~4 extra instructions just to tweak the error message you get.

[LegionMammal978]

If you want the shortest possible codegen, you can always map the error to a constant panic string:

pub fn compiler_bounds_check_mapped(p: &mut [u8], range: std::ops::Range<usize>) -> &mut [u8] {
    match p.get_mut(range) {
        Some(result) => result,
        None => panic!("out of bounds"),
    }
}

example::compiler_bounds_check_mapped:
        mov     r8, rcx
        sub     r8, rdx
        jb      .LBB9_2
        cmp     rcx, rsi
        ja      .LBB9_2
        add     rdi, rdx
        mov     rax, rdi
        mov     rdx, r8
        ret
.LBB9_2:
        push    rax
        call    std::panicking::begin_panic
        ud2

(Note that this is basically equivalent to expect()).

[steffahn]

start<l should already be ensured by the combination of end<l and start<=end, so that check seems redundant. Also you seem to be using the wrong pointer (p instead of start) and arguably that manual_bounds_check function should still be marked unsafe.

These things are probably all besides the main point of your question, but I noticed them and I'm leaving the forum for the day.

[scottmcm]

@jgarvin

assert!(range.start < l && range.end <= l && range.start <= range.end);

Well, you don't need to check all three of those -- the first one is superfluous. You can see what the library does here: https://doc.rust-lang.org/1.60.0/src/core/slice/index.rs.html#240-249

If you don't want the specific error, you could do something like p.get(range).expect("the range should have been valid").

Or, in general, you're best off assert!ing your function's precondition, as that can -- perhaps surprisingly -- result in fewer checks overall.

My go-to example of that is that this code has three checks:

pub fn add_first_three_simple(x: &[i32]) -> i32 {
    x[0] + x[1] + x[2]
}

but if you assert! the precondition, you end up with one check, not four:

pub fn add_first_three_asserted(x: &[i32]) -> i32 {
    assert!(x.len() >= 3);
    x[0] + x[1] + x[2]
}

Godbolt for the generated code: https://rust.godbolt.org/z/h3Ycjc39h

[jgarvin]

Oops yes I wrote the first two conditions originally and then discovered Range can't enforce start <= end since all the fields are pub.

[jgarvin]

@steffahn

Also you seem to be using the wrong pointer (p instead of start) and arguably that manual_bounds_check function should still be marked unsafe.

Good catch thanks