Skip to content

Commit 8f878c5

Browse files
committed
Auto merge of rust-lang#111010 - scottmcm:mem-replace-simpler, r=WaffleLapkin
Make `mem::replace` simpler in codegen Since they'd mentioned more intrinsics for simplifying stuff recently, r? `@WaffleLapkin` This is a continuation of me looking at foundational stuff that ends up with more instructions than it really needs. Specifically I noticed this one because `Range::next` isn't MIR-inlining, and one of the largest parts of it is a `replace::<usize>` that's a good dozen instructions instead of the two it could be. So this means that `ptr::write` with a `Copy` type no longer generates worse IR than manually dereferencing (well, at least in LLVM -- MIR still has bonus pointer casts), and in doing so means that we're finally down to just the two essential `memcpy`s when emitting `mem::replace` for a large type, rather than the bonus-`alloca` and three `memcpy`s we emitted before this ([or the 6 we currently emit in 1.69 stable](https://rust.godbolt.org/z/67W8on6nP)). That said, LLVM does _usually_ manage to optimize the extra code away. But it's still nice for it not to have to do as much, thanks to (for example) not going through an `alloca` when `replace`ing a primitive like a `usize`. (This is a new intrinsic, but one that's immediately lowered to existing MIR constructs, so not anything that MIRI or the codegen backends or MIR semantics needs to do work to handle.)
2 parents 6ffb970 + 0677d9b commit 8f878c5

File tree

2 files changed

+35
-12
lines changed

2 files changed

+35
-12
lines changed

core/src/intrinsics.rs

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2257,12 +2257,23 @@ extern "rust-intrinsic" {
22572257
/// This is an implementation detail of [`crate::ptr::read`] and should
22582258
/// not be used anywhere else. See its comments for why this exists.
22592259
///
2260-
/// This intrinsic can *only* be called where the argument is a local without
2261-
/// projections (`read_via_copy(p)`, not `read_via_copy(*p)`) so that it
2260+
/// This intrinsic can *only* be called where the pointer is a local without
2261+
/// projections (`read_via_copy(ptr)`, not `read_via_copy(*ptr)`) so that it
22622262
/// trivially obeys runtime-MIR rules about derefs in operands.
22632263
#[rustc_const_unstable(feature = "const_ptr_read", issue = "80377")]
22642264
#[rustc_nounwind]
2265-
pub fn read_via_copy<T>(p: *const T) -> T;
2265+
pub fn read_via_copy<T>(ptr: *const T) -> T;
2266+
2267+
/// This is an implementation detail of [`crate::ptr::write`] and should
2268+
/// not be used anywhere else. See its comments for why this exists.
2269+
///
2270+
/// This intrinsic can *only* be called where the pointer is a local without
2271+
/// projections (`write_via_move(ptr, x)`, not `write_via_move(*ptr, x)`) so
2272+
/// that it trivially obeys runtime-MIR rules about derefs in operands.
2273+
#[cfg(not(bootstrap))]
2274+
#[rustc_const_unstable(feature = "const_ptr_write", issue = "86302")]
2275+
#[rustc_nounwind]
2276+
pub fn write_via_move<T>(ptr: *mut T, value: T);
22662277

22672278
/// Returns the value of the discriminant for the variant in 'v';
22682279
/// if `T` has no discriminant, returns `0`.
@@ -2828,3 +2839,16 @@ pub const unsafe fn transmute_unchecked<Src, Dst>(src: Src) -> Dst {
28282839
// SAFETY: It's a transmute -- the caller promised it's fine.
28292840
unsafe { transmute_copy(&ManuallyDrop::new(src)) }
28302841
}
2842+
2843+
/// Polyfill for bootstrap
2844+
#[cfg(bootstrap)]
2845+
pub const unsafe fn write_via_move<T>(ptr: *mut T, value: T) {
2846+
use crate::mem::*;
2847+
// SAFETY: the caller must guarantee that `dst` is valid for writes.
2848+
// `dst` cannot overlap `src` because the caller has mutable access
2849+
// to `dst` while `src` is owned by this function.
2850+
unsafe {
2851+
copy_nonoverlapping::<T>(&value, ptr, 1);
2852+
forget(value);
2853+
}
2854+
}

core/src/ptr/mod.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1349,13 +1349,13 @@ pub const unsafe fn read_unaligned<T>(src: *const T) -> T {
13491349
#[rustc_const_unstable(feature = "const_ptr_write", issue = "86302")]
13501350
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
13511351
pub const unsafe fn write<T>(dst: *mut T, src: T) {
1352-
// We are calling the intrinsics directly to avoid function calls in the generated code
1353-
// as `intrinsics::copy_nonoverlapping` is a wrapper function.
1354-
extern "rust-intrinsic" {
1355-
#[rustc_const_stable(feature = "const_intrinsic_copy", since = "1.63.0")]
1356-
#[rustc_nounwind]
1357-
fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize);
1358-
}
1352+
// Semantically, it would be fine for this to be implemented as a
1353+
// `copy_nonoverlapping` and appropriate drop suppression of `src`.
1354+
1355+
// However, implementing via that currently produces more MIR than is ideal.
1356+
// Using an intrinsic keeps it down to just the simple `*dst = move src` in
1357+
// MIR (11 statements shorter, at the time of writing), and also allows
1358+
// `src` to stay an SSA value in codegen_ssa, rather than a memory one.
13591359

13601360
// SAFETY: the caller must guarantee that `dst` is valid for writes.
13611361
// `dst` cannot overlap `src` because the caller has mutable access
@@ -1365,8 +1365,7 @@ pub const unsafe fn write<T>(dst: *mut T, src: T) {
13651365
"ptr::write requires that the pointer argument is aligned and non-null",
13661366
[T](dst: *mut T) => is_aligned_and_not_null(dst)
13671367
);
1368-
copy_nonoverlapping(&src as *const T, dst, 1);
1369-
intrinsics::forget(src);
1368+
intrinsics::write_via_move(dst, src)
13701369
}
13711370
}
13721371

0 commit comments

Comments
 (0)