Skip to content

Commit 43a7802

Browse files
committed
Auto merge of rust-lang#110837 - scottmcm:offset-for-add, r=compiler-errors
Use MIR's `Offset` for pointer `add` too ~~Status: draft while waiting for rust-lang#110822 to land, since this is built atop that.~~ ~~r? `@ghost~~` Canonical Rust code has mostly moved to `add`/`sub` on pointers, which take `usize`, instead of `offset` which takes `isize`. (And, relatedly, when `sub_ptr` was added it turned out it replaced every single in-tree use of `offset_from`, because `usize` is just so much more useful than `isize` in Rust.) Unfortunately, `intrinsics::offset` could only accept `*const` and `isize`, so there's a *huge* amount of type conversions back and forth being done. They're identity conversions in the backend, but still end up producing quite a lot of unhelpful MIR. This PR changes `intrinsics::offset` to accept `*const` *and* `*mut` along with `isize` *and* `usize`. Conveniently, the backends and CTFE already handle this, since MIR's `BinOp::Offset` [already supports all four combinations](https://github.com/rust-lang/rust/blob/adaac6b166df57ea5a20d56e4cce503b55aca927/compiler/rustc_const_eval/src/transform/validate.rs#L523-L528). To demonstrate the difference, I added some `mir-opt/pre-codegen/` tests around slice indexing. Here's the difference to `[T]::get_mut`, since it uses `<*mut _>::add` internally: ```diff `@@` -79,30 +70,21 `@@` fn slice_get_mut_usize(_1: &mut [u32], _2: usize) -> Option<&mut u32> { StorageLive(_12); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL StorageLive(_9); // scope 6 at $SRC_DIR/core/src/slice/index.rs:LL:COL _9 = _8 as *mut u32 (PtrToPtr); // scope 11 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL - StorageLive(_13); // scope 13 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL - _13 = _2 as isize (IntToInt); // scope 13 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL - StorageLive(_14); // scope 15 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL - StorageLive(_15); // scope 15 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL - _15 = _9 as *const u32 (Pointer(MutToConstPointer)); // scope 15 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL - _14 = Offset(move _15, _13); // scope 15 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL - StorageDead(_15); // scope 15 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL - _7 = move _14 as *mut u32 (PtrToPtr); // scope 15 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL - StorageDead(_14); // scope 15 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL - StorageDead(_13); // scope 13 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL + _7 = Offset(_9, _2); // scope 13 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL StorageDead(_9); // scope 6 at $SRC_DIR/core/src/slice/index.rs:LL:COL StorageDead(_12); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL StorageDead(_11); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL ``` rust-lang@1c1c8e4#diff-a841b6a4538657add3f39bc895744331453d0625e7aace128b1f604f0b63c8fdR80
2 parents 2fce229 + e1da77c commit 43a7802

14 files changed

+401
-13
lines changed

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

+9-2
Original file line numberDiff line numberDiff line change
@@ -819,8 +819,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
819819
.builtin_deref(true)
820820
.unwrap_or_else(|| bug!("deref of non-pointer {:?}", input_ty))
821821
.ty;
822-
let llty = bx.cx().backend_type(bx.cx().layout_of(pointee_type));
823-
bx.inbounds_gep(llty, lhs, &[rhs])
822+
let pointee_layout = bx.cx().layout_of(pointee_type);
823+
if pointee_layout.is_zst() {
824+
// `Offset` works in terms of the size of pointee,
825+
// so offsetting a pointer to ZST is a noop.
826+
lhs
827+
} else {
828+
let llty = bx.cx().backend_type(pointee_layout);
829+
bx.inbounds_gep(llty, lhs, &[rhs])
830+
}
824831
}
825832
mir::BinOp::Shl => common::build_unchecked_lshift(bx, lhs, rhs),
826833
mir::BinOp::Shr => common::build_unchecked_rshift(bx, input_ty, lhs, rhs),

compiler/rustc_hir_analysis/src/check/intrinsic.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,8 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
215215

216216
sym::type_name => (1, Vec::new(), tcx.mk_static_str()),
217217
sym::type_id => (1, Vec::new(), tcx.types.u64),
218-
sym::offset | sym::arith_offset => (
218+
sym::offset => (2, vec![param(0), param(1)], param(0)),
219+
sym::arith_offset => (
219220
1,
220221
vec![
221222
tcx.mk_ptr(ty::TypeAndMut { ty: param(0), mutbl: hir::Mutability::Not }),

library/core/src/intrinsics.rs

+12
Original file line numberDiff line numberDiff line change
@@ -1413,6 +1413,10 @@ extern "rust-intrinsic" {
14131413
/// This is implemented as an intrinsic to avoid converting to and from an
14141414
/// integer, since the conversion would throw away aliasing information.
14151415
///
1416+
/// This can only be used with `Ptr` as a raw pointer type (`*mut` or `*const`)
1417+
/// to a `Sized` pointee and with `Delta` as `usize` or `isize`. Any other
1418+
/// instantiations may arbitrarily misbehave, and that's *not* a compiler bug.
1419+
///
14161420
/// # Safety
14171421
///
14181422
/// Both the starting and resulting pointer must be either in bounds or one
@@ -1421,6 +1425,14 @@ extern "rust-intrinsic" {
14211425
/// returned value will result in undefined behavior.
14221426
///
14231427
/// The stabilized version of this intrinsic is [`pointer::offset`].
1428+
#[cfg(not(bootstrap))]
1429+
#[must_use = "returns a new pointer rather than modifying its argument"]
1430+
#[rustc_const_stable(feature = "const_ptr_offset", since = "1.61.0")]
1431+
#[rustc_nounwind]
1432+
pub fn offset<Ptr, Delta>(dst: Ptr, offset: Delta) -> Ptr;
1433+
1434+
/// The bootstrap version of this is more restricted.
1435+
#[cfg(bootstrap)]
14241436
#[must_use = "returns a new pointer rather than modifying its argument"]
14251437
#[rustc_const_stable(feature = "const_ptr_offset", since = "1.61.0")]
14261438
#[rustc_nounwind]

library/core/src/ptr/const_ptr.rs

+9-1
Original file line numberDiff line numberDiff line change
@@ -916,8 +916,16 @@ impl<T: ?Sized> *const T {
916916
where
917917
T: Sized,
918918
{
919+
#[cfg(bootstrap)]
919920
// SAFETY: the caller must uphold the safety contract for `offset`.
920-
unsafe { self.offset(count as isize) }
921+
unsafe {
922+
self.offset(count as isize)
923+
}
924+
#[cfg(not(bootstrap))]
925+
// SAFETY: the caller must uphold the safety contract for `offset`.
926+
unsafe {
927+
intrinsics::offset(self, count)
928+
}
921929
}
922930

923931
/// Calculates the offset from a pointer in bytes (convenience for `.byte_offset(count as isize)`).

library/core/src/ptr/mut_ptr.rs

+20-2
Original file line numberDiff line numberDiff line change
@@ -473,10 +473,20 @@ impl<T: ?Sized> *mut T {
473473
where
474474
T: Sized,
475475
{
476+
#[cfg(bootstrap)]
476477
// SAFETY: the caller must uphold the safety contract for `offset`.
477478
// The obtained pointer is valid for writes since the caller must
478479
// guarantee that it points to the same allocated object as `self`.
479-
unsafe { intrinsics::offset(self, count) as *mut T }
480+
unsafe {
481+
intrinsics::offset(self, count) as *mut T
482+
}
483+
#[cfg(not(bootstrap))]
484+
// SAFETY: the caller must uphold the safety contract for `offset`.
485+
// The obtained pointer is valid for writes since the caller must
486+
// guarantee that it points to the same allocated object as `self`.
487+
unsafe {
488+
intrinsics::offset(self, count)
489+
}
480490
}
481491

482492
/// Calculates the offset from a pointer in bytes.
@@ -1016,8 +1026,16 @@ impl<T: ?Sized> *mut T {
10161026
where
10171027
T: Sized,
10181028
{
1029+
#[cfg(bootstrap)]
1030+
// SAFETY: the caller must uphold the safety contract for `offset`.
1031+
unsafe {
1032+
self.offset(count as isize)
1033+
}
1034+
#[cfg(not(bootstrap))]
10191035
// SAFETY: the caller must uphold the safety contract for `offset`.
1020-
unsafe { self.offset(count as isize) }
1036+
unsafe {
1037+
intrinsics::offset(self, count)
1038+
}
10211039
}
10221040

10231041
/// Calculates the offset from a pointer in bytes (convenience for `.byte_offset(count as isize)`).

src/doc/unstable-book/src/language-features/intrinsics.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ via a declaration like
2222
extern "rust-intrinsic" {
2323
fn transmute<T, U>(x: T) -> U;
2424

25-
fn offset<T>(dst: *const T, offset: isize) -> *const T;
25+
fn arith_offset<T>(dst: *const T, offset: isize) -> *const T;
2626
}
2727
```
2828

tests/codegen/intrinsics/offset.rs

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// compile-flags: -O -C no-prepopulate-passes
2+
// min-llvm-version: 15.0 (because we're using opaque pointers)
3+
4+
#![crate_type = "lib"]
5+
#![feature(core_intrinsics)]
6+
7+
use std::intrinsics::offset;
8+
9+
// CHECK-LABEL: ptr @offset_zst
10+
// CHECK-SAME: (ptr noundef %p, [[SIZE:i[0-9]+]] noundef %d)
11+
#[no_mangle]
12+
pub unsafe fn offset_zst(p: *const (), d: usize) -> *const () {
13+
// CHECK-NOT: getelementptr
14+
// CHECK: ret ptr %p
15+
offset(p, d)
16+
}
17+
18+
// CHECK-LABEL: ptr @offset_isize
19+
// CHECK-SAME: (ptr noundef %p, [[SIZE]] noundef %d)
20+
#[no_mangle]
21+
pub unsafe fn offset_isize(p: *const u32, d: isize) -> *const u32 {
22+
// CHECK: %[[R:.*]] = getelementptr inbounds i32, ptr %p, [[SIZE]] %d
23+
// CHECK-NEXT: ret ptr %[[R]]
24+
offset(p, d)
25+
}
26+
27+
// CHECK-LABEL: ptr @offset_usize
28+
// CHECK-SAME: (ptr noundef %p, [[SIZE]] noundef %d)
29+
#[no_mangle]
30+
pub unsafe fn offset_usize(p: *const u64, d: usize) -> *const u64 {
31+
// CHECK: %[[R:.*]] = getelementptr inbounds i64, ptr %p, [[SIZE]] %d
32+
// CHECK-NEXT: ret ptr %[[R]]
33+
offset(p, d)
34+
}

tests/mir-opt/lower_intrinsics.ptr_offset.LowerIntrinsics.diff

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@
1313
_3 = _1; // scope 0 at $DIR/lower_intrinsics.rs:+1:30: +1:31
1414
StorageLive(_4); // scope 0 at $DIR/lower_intrinsics.rs:+1:33: +1:34
1515
_4 = _2; // scope 0 at $DIR/lower_intrinsics.rs:+1:33: +1:34
16-
- _0 = offset::<i32>(move _3, move _4) -> [return: bb1, unwind unreachable]; // scope 0 at $DIR/lower_intrinsics.rs:+1:5: +1:35
16+
- _0 = offset::<*const i32, isize>(move _3, move _4) -> [return: bb1, unwind unreachable]; // scope 0 at $DIR/lower_intrinsics.rs:+1:5: +1:35
1717
- // mir::Constant
1818
- // + span: $DIR/lower_intrinsics.rs:140:5: 140:29
19-
- // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(*const i32, isize) -> *const i32 {offset::<i32>}, val: Value(<ZST>) }
19+
- // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(*const i32, isize) -> *const i32 {offset::<*const i32, isize>}, val: Value(<ZST>) }
2020
+ _0 = Offset(move _3, move _4); // scope 0 at $DIR/lower_intrinsics.rs:+1:5: +1:35
2121
+ goto -> bb1; // scope 0 at $DIR/lower_intrinsics.rs:+1:5: +1:35
2222
}
+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2
2+
// only-64bit
3+
// ignore-debug
4+
5+
#![crate_type = "lib"]
6+
7+
use std::ops::Range;
8+
9+
// EMIT_MIR slice_index.slice_index_usize.PreCodegen.after.mir
10+
pub fn slice_index_usize(slice: &[u32], index: usize) -> u32 {
11+
slice[index]
12+
}
13+
14+
// EMIT_MIR slice_index.slice_get_mut_usize.PreCodegen.after.mir
15+
pub fn slice_get_mut_usize(slice: &mut [u32], index: usize) -> Option<&mut u32> {
16+
slice.get_mut(index)
17+
}
18+
19+
// EMIT_MIR slice_index.slice_index_range.PreCodegen.after.mir
20+
pub fn slice_index_range(slice: &[u32], index: Range<usize>) -> &[u32] {
21+
&slice[index]
22+
}
23+
24+
// EMIT_MIR slice_index.slice_get_unchecked_mut_range.PreCodegen.after.mir
25+
pub unsafe fn slice_get_unchecked_mut_range(slice: &mut [u32], index: Range<usize>) -> &mut [u32] {
26+
slice.get_unchecked_mut(index)
27+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// MIR for `slice_get_mut_usize` after PreCodegen
2+
3+
fn slice_get_mut_usize(_1: &mut [u32], _2: usize) -> Option<&mut u32> {
4+
debug slice => _1; // in scope 0 at $DIR/slice_index.rs:+0:28: +0:33
5+
debug index => _2; // in scope 0 at $DIR/slice_index.rs:+0:47: +0:52
6+
let mut _0: std::option::Option<&mut u32>; // return place in scope 0 at $DIR/slice_index.rs:+0:64: +0:80
7+
scope 1 (inlined core::slice::<impl [u32]>::get_mut::<usize>) { // at $DIR/slice_index.rs:16:11: 16:25
8+
debug self => _1; // in scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL
9+
debug index => _2; // in scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL
10+
scope 2 (inlined <usize as SliceIndex<[u32]>>::get_mut) { // at $SRC_DIR/core/src/slice/mod.rs:LL:COL
11+
debug self => _2; // in scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
12+
debug slice => _1; // in scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
13+
let mut _3: bool; // in scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
14+
let mut _4: usize; // in scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
15+
let mut _5: &[u32]; // in scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
16+
let mut _6: &mut u32; // in scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
17+
let mut _7: *mut u32; // in scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
18+
let mut _8: *mut [u32]; // in scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
19+
scope 3 {
20+
scope 4 (inlined <usize as SliceIndex<[u32]>>::get_unchecked_mut) { // at $SRC_DIR/core/src/slice/index.rs:LL:COL
21+
debug self => _2; // in scope 4 at $SRC_DIR/core/src/slice/index.rs:LL:COL
22+
debug slice => _8; // in scope 4 at $SRC_DIR/core/src/slice/index.rs:LL:COL
23+
let mut _9: *mut u32; // in scope 4 at $SRC_DIR/core/src/slice/index.rs:LL:COL
24+
let mut _10: usize; // in scope 4 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
25+
let mut _11: *mut [u32]; // in scope 4 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
26+
scope 5 {
27+
debug this => _2; // in scope 5 at $SRC_DIR/core/src/slice/index.rs:LL:COL
28+
scope 6 {
29+
scope 7 (inlined <usize as SliceIndex<[T]>>::get_unchecked_mut::runtime::<u32>) { // at $SRC_DIR/core/src/intrinsics.rs:LL:COL
30+
debug this => _10; // in scope 7 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
31+
debug slice => _11; // in scope 7 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
32+
scope 8 (inlined ptr::mut_ptr::<impl *mut [u32]>::len) { // at $SRC_DIR/core/src/slice/index.rs:LL:COL
33+
debug self => _11; // in scope 8 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
34+
let mut _12: *const [u32]; // in scope 8 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
35+
scope 9 (inlined std::ptr::metadata::<[u32]>) { // at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
36+
debug ptr => _12; // in scope 9 at $SRC_DIR/core/src/ptr/metadata.rs:LL:COL
37+
scope 10 {
38+
}
39+
}
40+
}
41+
}
42+
scope 11 (inlined ptr::mut_ptr::<impl *mut [u32]>::as_mut_ptr) { // at $SRC_DIR/core/src/slice/index.rs:LL:COL
43+
debug self => _8; // in scope 11 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
44+
}
45+
scope 12 (inlined ptr::mut_ptr::<impl *mut u32>::add) { // at $SRC_DIR/core/src/slice/index.rs:LL:COL
46+
debug self => _9; // in scope 12 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
47+
debug count => _2; // in scope 12 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
48+
scope 13 {
49+
}
50+
}
51+
}
52+
}
53+
}
54+
}
55+
}
56+
}
57+
58+
bb0: {
59+
StorageLive(_6); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL
60+
StorageLive(_3); // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
61+
StorageLive(_4); // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
62+
StorageLive(_5); // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
63+
_5 = &(*_1); // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
64+
_4 = Len((*_5)); // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
65+
StorageDead(_5); // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
66+
_3 = Lt(_2, move _4); // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
67+
StorageDead(_4); // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
68+
switchInt(move _3) -> [0: bb2, otherwise: bb1]; // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
69+
}
70+
71+
bb1: {
72+
StorageLive(_7); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL
73+
StorageLive(_8); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL
74+
_8 = &raw mut (*_1); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL
75+
StorageLive(_10); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL
76+
StorageLive(_11); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL
77+
StorageLive(_12); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL
78+
StorageLive(_9); // scope 6 at $SRC_DIR/core/src/slice/index.rs:LL:COL
79+
_9 = _8 as *mut u32 (PtrToPtr); // scope 11 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
80+
_7 = Offset(_9, _2); // scope 13 at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
81+
StorageDead(_9); // scope 6 at $SRC_DIR/core/src/slice/index.rs:LL:COL
82+
StorageDead(_12); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL
83+
StorageDead(_11); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL
84+
StorageDead(_10); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL
85+
StorageDead(_8); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL
86+
_6 = &mut (*_7); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL
87+
_0 = Option::<&mut u32>::Some(_6); // scope 3 at $SRC_DIR/core/src/slice/index.rs:LL:COL
88+
StorageDead(_7); // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
89+
goto -> bb3; // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
90+
}
91+
92+
bb2: {
93+
_0 = const Option::<&mut u32>::None; // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
94+
// mir::Constant
95+
// + span: no-location
96+
// + literal: Const { ty: Option<&mut u32>, val: Value(Scalar(0x0000000000000000)) }
97+
goto -> bb3; // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
98+
}
99+
100+
bb3: {
101+
StorageDead(_3); // scope 2 at $SRC_DIR/core/src/slice/index.rs:LL:COL
102+
StorageDead(_6); // scope 1 at $SRC_DIR/core/src/slice/mod.rs:LL:COL
103+
return; // scope 0 at $DIR/slice_index.rs:+2:2: +2:2
104+
}
105+
}

0 commit comments

Comments
 (0)