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:
- Manual does three separate comparisons (worse)
- 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.
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()
).
1 Like
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.
1 Like
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
3 Likes
Oops yes I wrote the first two conditions originally and then discovered Range can't enforce start <= end
since all the fields are pub.