Skip to content

Commit f0999ff

Browse files
committed
Auto merge of #139118 - scottmcm:slice-get-unchecked-intrinsic, r=workingjubilee
`slice.get(i)` should use a slice projection in MIR, like `slice[i]` does `slice[i]` is built-in magic, so ends up being quite different from `slice.get(i)` in MIR, even though they're both doing nearly identical operations -- checking the length of the slice then getting a ref/ptr to the element if it's in-bounds. This PR adds a `slice_get_unchecked` intrinsic for `impl SliceIndex for usize` to use to fix that, so it no longer needs to do a bunch of lines of pointer math and instead just gets the obvious single statement. (This is *not* used for the range versions, since `slice[i..]` and `slice[..k]` can't use the mir Slice projection as they're using fenceposts, not indices.) I originally tried to do this with some kind of GVN pattern, but realized that I'm pretty sure it's not legal to optimize `BinOp::Offset` to `PlaceElem::Index` without an extremely complicated condition. Basically, the problem is that the `Index` projection on a dereferenced slice pointer *cares about the metadata*, since it's UB to `PlaceElem::Index` outside the range described by the metadata. But then you cast the fat pointer to a thin pointer then offset it, that *ignores* the slice length metadata, so it's possible to write things that are legal with `Offset` but would be UB if translated in the obvious way to `Index`. Checking (or even determining) the necessary conditions for that would be complicated and error-prone, whereas this intrinsic-based approach is quite straight-forward. Zero backend changes, because it just lowers to MIR, so it's already supported naturally by CTFE/Miri/cg_llvm/cg_clif.
2 parents 4d08223 + 4668124 commit f0999ff

15 files changed

+385
-86
lines changed

compiler/rustc_hir_analysis/src/check/intrinsic.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ pub(crate) fn check_intrinsic_type(
265265
vec![Ty::new_imm_ptr(tcx, param(0)), tcx.types.isize],
266266
Ty::new_imm_ptr(tcx, param(0)),
267267
),
268+
sym::slice_get_unchecked => (3, 0, vec![param(1), tcx.types.usize], param(0)),
268269
sym::ptr_mask => (
269270
1,
270271
0,

compiler/rustc_mir_transform/src/lower_intrinsics.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,52 @@ impl<'tcx> crate::MirPass<'tcx> for LowerIntrinsics {
262262
});
263263
terminator.kind = TerminatorKind::Goto { target };
264264
}
265+
sym::slice_get_unchecked => {
266+
let target = target.unwrap();
267+
let Ok([ptrish, index]) = take_array(args) else {
268+
span_bug!(
269+
terminator.source_info.span,
270+
"Wrong number of arguments for {intrinsic:?}",
271+
);
272+
};
273+
274+
let place = ptrish.node.place().unwrap();
275+
assert!(!place.is_indirect());
276+
let updated_place = place.project_deeper(
277+
&[
278+
ProjectionElem::Deref,
279+
ProjectionElem::Index(
280+
index.node.place().unwrap().as_local().unwrap(),
281+
),
282+
],
283+
tcx,
284+
);
285+
286+
let ret_ty = generic_args.type_at(0);
287+
let rvalue = match *ret_ty.kind() {
288+
ty::RawPtr(_, Mutability::Not) => {
289+
Rvalue::RawPtr(RawPtrKind::Const, updated_place)
290+
}
291+
ty::RawPtr(_, Mutability::Mut) => {
292+
Rvalue::RawPtr(RawPtrKind::Mut, updated_place)
293+
}
294+
ty::Ref(region, _, Mutability::Not) => {
295+
Rvalue::Ref(region, BorrowKind::Shared, updated_place)
296+
}
297+
ty::Ref(region, _, Mutability::Mut) => Rvalue::Ref(
298+
region,
299+
BorrowKind::Mut { kind: MutBorrowKind::Default },
300+
updated_place,
301+
),
302+
_ => bug!("Unknown return type {ret_ty:?}"),
303+
};
304+
305+
block.statements.push(Statement {
306+
source_info: terminator.source_info,
307+
kind: StatementKind::Assign(Box::new((*destination, rvalue))),
308+
});
309+
terminator.kind = TerminatorKind::Goto { target };
310+
}
265311
sym::transmute | sym::transmute_unchecked => {
266312
let dst_ty = destination.ty(local_decls, tcx).ty;
267313
let Ok([arg]) = take_array(args) else {

compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2000,6 +2000,7 @@ symbols! {
20002000
slice,
20012001
slice_from_raw_parts,
20022002
slice_from_raw_parts_mut,
2003+
slice_get_unchecked,
20032004
slice_into_vec,
20042005
slice_iter,
20052006
slice_len_fn,

library/core/src/intrinsics/bounds.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
//! Various traits used to restrict intrinsics to not-completely-wrong types.
2+
3+
/// Types with a built-in dereference operator in runtime MIR,
4+
/// aka references and raw pointers.
5+
///
6+
/// # Safety
7+
/// Must actually *be* such a type.
8+
pub unsafe trait BuiltinDeref: Sized {
9+
type Pointee: ?Sized;
10+
}
11+
12+
unsafe impl<T: ?Sized> BuiltinDeref for &mut T {
13+
type Pointee = T;
14+
}
15+
unsafe impl<T: ?Sized> BuiltinDeref for &T {
16+
type Pointee = T;
17+
}
18+
unsafe impl<T: ?Sized> BuiltinDeref for *mut T {
19+
type Pointee = T;
20+
}
21+
unsafe impl<T: ?Sized> BuiltinDeref for *const T {
22+
type Pointee = T;
23+
}
24+
25+
pub trait ChangePointee<U: ?Sized>: BuiltinDeref {
26+
type Output;
27+
}
28+
impl<'a, T: ?Sized + 'a, U: ?Sized + 'a> ChangePointee<U> for &'a mut T {
29+
type Output = &'a mut U;
30+
}
31+
impl<'a, T: ?Sized + 'a, U: ?Sized + 'a> ChangePointee<U> for &'a T {
32+
type Output = &'a U;
33+
}
34+
impl<T: ?Sized, U: ?Sized> ChangePointee<U> for *mut T {
35+
type Output = *mut U;
36+
}
37+
impl<T: ?Sized, U: ?Sized> ChangePointee<U> for *const T {
38+
type Output = *const U;
39+
}

library/core/src/intrinsics/mod.rs

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
use crate::marker::{ConstParamTy, DiscriminantKind, Tuple};
5454
use crate::ptr;
5555

56+
mod bounds;
5657
pub mod fallback;
5758
pub mod mir;
5859
pub mod simd;
@@ -1730,7 +1731,7 @@ pub const fn needs_drop<T: ?Sized>() -> bool;
17301731
#[rustc_intrinsic_const_stable_indirect]
17311732
#[rustc_nounwind]
17321733
#[rustc_intrinsic]
1733-
pub const unsafe fn offset<Ptr, Delta>(dst: Ptr, offset: Delta) -> Ptr;
1734+
pub const unsafe fn offset<Ptr: bounds::BuiltinDeref, Delta>(dst: Ptr, offset: Delta) -> Ptr;
17341735

17351736
/// Calculates the offset from a pointer, potentially wrapping.
17361737
///
@@ -1751,6 +1752,33 @@ pub const unsafe fn offset<Ptr, Delta>(dst: Ptr, offset: Delta) -> Ptr;
17511752
#[rustc_intrinsic]
17521753
pub const unsafe fn arith_offset<T>(dst: *const T, offset: isize) -> *const T;
17531754

1755+
/// Projects to the `index`-th element of `slice_ptr`, as the same kind of pointer
1756+
/// as the slice was provided -- so `&mut [T] → &mut T`, `&[T] → &T`,
1757+
/// `*mut [T] → *mut T`, or `*const [T] → *const T` -- without a bounds check.
1758+
///
1759+
/// This is exposed via `<usize as SliceIndex>::get(_unchecked)(_mut)`,
1760+
/// and isn't intended to be used elsewhere.
1761+
///
1762+
/// Expands in MIR to `{&, &mut, &raw const, &raw mut} (*slice_ptr)[index]`,
1763+
/// depending on the types involved, so no backend support is needed.
1764+
///
1765+
/// # Safety
1766+
///
1767+
/// - `index < PtrMetadata(slice_ptr)`, so the indexing is in-bounds for the slice
1768+
/// - the resulting offsetting is in-bounds of the allocated object, which is
1769+
/// always the case for references, but needs to be upheld manually for pointers
1770+
#[cfg(not(bootstrap))]
1771+
#[rustc_nounwind]
1772+
#[rustc_intrinsic]
1773+
pub const unsafe fn slice_get_unchecked<
1774+
ItemPtr: bounds::ChangePointee<[T], Pointee = T, Output = SlicePtr>,
1775+
SlicePtr,
1776+
T,
1777+
>(
1778+
slice_ptr: SlicePtr,
1779+
index: usize,
1780+
) -> ItemPtr;
1781+
17541782
/// Masks out bits of the pointer according to a mask.
17551783
///
17561784
/// Note that, unlike most intrinsics, this is safe to call;
@@ -3575,18 +3603,9 @@ pub const fn type_id<T: ?Sized + 'static>() -> u128;
35753603
#[unstable(feature = "core_intrinsics", issue = "none")]
35763604
#[rustc_intrinsic_const_stable_indirect]
35773605
#[rustc_intrinsic]
3578-
pub const fn aggregate_raw_ptr<P: AggregateRawPtr<D, Metadata = M>, D, M>(data: D, meta: M) -> P;
3579-
3580-
#[unstable(feature = "core_intrinsics", issue = "none")]
3581-
pub trait AggregateRawPtr<D> {
3582-
type Metadata: Copy;
3583-
}
3584-
impl<P: ?Sized, T: ptr::Thin> AggregateRawPtr<*const T> for *const P {
3585-
type Metadata = <P as ptr::Pointee>::Metadata;
3586-
}
3587-
impl<P: ?Sized, T: ptr::Thin> AggregateRawPtr<*mut T> for *mut P {
3588-
type Metadata = <P as ptr::Pointee>::Metadata;
3589-
}
3606+
pub const fn aggregate_raw_ptr<P: bounds::BuiltinDeref, D, M>(data: D, meta: M) -> P
3607+
where
3608+
<P as bounds::BuiltinDeref>::Pointee: ptr::Pointee<Metadata = M>;
35903609

35913610
/// Lowers in MIR to `Rvalue::UnaryOp` with `UnOp::PtrMetadata`.
35923611
///

library/core/src/slice/index.rs

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
//! Indexing implementations for `[T]`.
22
3+
#[cfg(not(bootstrap))]
4+
use crate::intrinsics::slice_get_unchecked;
35
use crate::panic::const_panic;
46
use crate::ub_checks::assert_unsafe_precondition;
57
use crate::{ops, range};
@@ -83,13 +85,15 @@ const fn slice_end_index_overflow_fail() -> ! {
8385
// Both the safe and unsafe public methods share these helpers,
8486
// which use intrinsics directly to get *no* extra checks.
8587

88+
#[cfg(bootstrap)]
8689
#[inline(always)]
8790
const unsafe fn get_noubcheck<T>(ptr: *const [T], index: usize) -> *const T {
8891
let ptr = ptr as *const T;
8992
// SAFETY: The caller already checked these preconditions
9093
unsafe { crate::intrinsics::offset(ptr, index) }
9194
}
9295

96+
#[cfg(bootstrap)]
9397
#[inline(always)]
9498
const unsafe fn get_mut_noubcheck<T>(ptr: *mut [T], index: usize) -> *mut T {
9599
let ptr = ptr as *mut T;
@@ -103,8 +107,9 @@ const unsafe fn get_offset_len_noubcheck<T>(
103107
offset: usize,
104108
len: usize,
105109
) -> *const [T] {
110+
let ptr = ptr as *const T;
106111
// SAFETY: The caller already checked these preconditions
107-
let ptr = unsafe { get_noubcheck(ptr, offset) };
112+
let ptr = unsafe { crate::intrinsics::offset(ptr, offset) };
108113
crate::intrinsics::aggregate_raw_ptr(ptr, len)
109114
}
110115

@@ -114,8 +119,9 @@ const unsafe fn get_offset_len_mut_noubcheck<T>(
114119
offset: usize,
115120
len: usize,
116121
) -> *mut [T] {
122+
let ptr = ptr as *mut T;
117123
// SAFETY: The caller already checked these preconditions
118-
let ptr = unsafe { get_mut_noubcheck(ptr, offset) };
124+
let ptr = unsafe { crate::intrinsics::offset(ptr, offset) };
119125
crate::intrinsics::aggregate_raw_ptr(ptr, len)
120126
}
121127

@@ -224,15 +230,35 @@ unsafe impl<T> SliceIndex<[T]> for usize {
224230

225231
#[inline]
226232
fn get(self, slice: &[T]) -> Option<&T> {
227-
// SAFETY: `self` is checked to be in bounds.
228-
if self < slice.len() { unsafe { Some(&*get_noubcheck(slice, self)) } } else { None }
233+
if self < slice.len() {
234+
#[cfg(bootstrap)]
235+
// SAFETY: `self` is checked to be in bounds.
236+
unsafe {
237+
Some(&*get_noubcheck(slice, self))
238+
}
239+
#[cfg(not(bootstrap))]
240+
// SAFETY: `self` is checked to be in bounds.
241+
unsafe {
242+
Some(slice_get_unchecked(slice, self))
243+
}
244+
} else {
245+
None
246+
}
229247
}
230248

231249
#[inline]
232250
fn get_mut(self, slice: &mut [T]) -> Option<&mut T> {
233251
if self < slice.len() {
252+
#[cfg(bootstrap)]
253+
// SAFETY: `self` is checked to be in bounds.
254+
unsafe {
255+
Some(&mut *get_mut_noubcheck(slice, self))
256+
}
257+
#[cfg(not(bootstrap))]
234258
// SAFETY: `self` is checked to be in bounds.
235-
unsafe { Some(&mut *get_mut_noubcheck(slice, self)) }
259+
unsafe {
260+
Some(slice_get_unchecked(slice, self))
261+
}
236262
} else {
237263
None
238264
}
@@ -254,7 +280,14 @@ unsafe impl<T> SliceIndex<[T]> for usize {
254280
// Use intrinsics::assume instead of hint::assert_unchecked so that we don't check the
255281
// precondition of this function twice.
256282
crate::intrinsics::assume(self < slice.len());
257-
get_noubcheck(slice, self)
283+
#[cfg(bootstrap)]
284+
{
285+
get_noubcheck(slice, self)
286+
}
287+
#[cfg(not(bootstrap))]
288+
{
289+
slice_get_unchecked(slice, self)
290+
}
258291
}
259292
}
260293

@@ -267,7 +300,16 @@ unsafe impl<T> SliceIndex<[T]> for usize {
267300
(this: usize = self, len: usize = slice.len()) => this < len
268301
);
269302
// SAFETY: see comments for `get_unchecked` above.
270-
unsafe { get_mut_noubcheck(slice, self) }
303+
unsafe {
304+
#[cfg(bootstrap)]
305+
{
306+
get_mut_noubcheck(slice, self)
307+
}
308+
#[cfg(not(bootstrap))]
309+
{
310+
slice_get_unchecked(slice, self)
311+
}
312+
}
271313
}
272314

273315
#[inline]

tests/mir-opt/lower_intrinsics.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,3 +263,24 @@ pub fn get_metadata(a: *const i32, b: *const [u8], c: *const dyn std::fmt::Debug
263263
let _usize = ptr_metadata(b);
264264
let _vtable = ptr_metadata(c);
265265
}
266+
267+
// EMIT_MIR lower_intrinsics.slice_get.LowerIntrinsics.diff
268+
pub unsafe fn slice_get<'a, 'b>(
269+
r: &'a [i8],
270+
rm: &'b mut [i16],
271+
p: *const [i32],
272+
pm: *mut [i64],
273+
i: usize,
274+
) -> (&'a i8, &'b mut i16, *const i32, *mut i64) {
275+
use std::intrinsics::slice_get_unchecked;
276+
// CHECK: = &(*_{{[0-9]+}})[_{{[0-9]+}}]
277+
// CHECK: = &mut (*_{{[0-9]+}})[_{{[0-9]+}}]
278+
// CHECK: = &raw const (*_{{[0-9]+}})[_{{[0-9]+}}]
279+
// CHECK: = &raw mut (*_{{[0-9]+}})[_{{[0-9]+}}]
280+
(
281+
slice_get_unchecked(r, i),
282+
slice_get_unchecked(rm, i),
283+
slice_get_unchecked(p, i),
284+
slice_get_unchecked(pm, i),
285+
)
286+
}

0 commit comments

Comments
 (0)