diff options
Diffstat (limited to 'library/core/src/ptr/mod.rs')
| -rw-r--r-- | library/core/src/ptr/mod.rs | 59 |
1 files changed, 45 insertions, 14 deletions
diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index f41be46abc9..5884a8ca308 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -1135,27 +1135,58 @@ pub const unsafe fn replace<T>(dst: *mut T, mut src: T) -> T { #[rustc_const_unstable(feature = "const_ptr_read", issue = "80377")] #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces pub const unsafe fn read<T>(src: *const T) -> T { - // We are calling the intrinsics directly to avoid function calls in the generated code - // as `intrinsics::copy_nonoverlapping` is a wrapper function. - extern "rust-intrinsic" { - #[rustc_const_stable(feature = "const_intrinsic_copy", since = "1.63.0")] - fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize); - } + // It would be semantically correct to implement this via `copy_nonoverlapping` + // and `MaybeUninit`, as was done before PR #109035. Calling `assume_init` + // provides enough information to know that this is a typed operation. - let mut tmp = MaybeUninit::<T>::uninit(); - // SAFETY: the caller must guarantee that `src` is valid for reads. - // `src` cannot overlap `tmp` because `tmp` was just allocated on - // the stack as a separate allocated object. + // However, as of March 2023 the compiler was not capable of taking advantage + // of that information. Thus the implementation here switched to an intrinsic, + // which lowers to `_0 = *src` in MIR, to address a few issues: // - // Also, since we just wrote a valid value into `tmp`, it is guaranteed - // to be properly initialized. + // - Using `MaybeUninit::assume_init` after a `copy_nonoverlapping` was not + // turning the untyped copy into a typed load. As such, the generated + // `load` in LLVM didn't get various metadata, such as `!range` (#73258), + // `!nonnull`, and `!noundef`, resulting in poorer optimization. + // - Going through the extra local resulted in multiple extra copies, even + // in optimized MIR. (Ignoring StorageLive/Dead, the intrinsic is one + // MIR statement, while the previous implementation was eight.) LLVM + // could sometimes optimize them away, but because `read` is at the core + // of so many things, not having them in the first place improves what we + // hand off to the backend. For example, `mem::replace::<Big>` previously + // emitted 4 `alloca` and 6 `memcpy`s, but is now 1 `alloc` and 3 `memcpy`s. + // - In general, this approach keeps us from getting any more bugs (like + // #106369) that boil down to "`read(p)` is worse than `*p`", as this + // makes them look identical to the backend (or other MIR consumers). + // + // Future enhancements to MIR optimizations might well allow this to return + // to the previous implementation, rather than using an intrinsic. + + // SAFETY: the caller must guarantee that `src` is valid for reads. unsafe { assert_unsafe_precondition!( "ptr::read requires that the pointer argument is aligned and non-null", [T](src: *const T) => is_aligned_and_not_null(src) ); - copy_nonoverlapping(src, tmp.as_mut_ptr(), 1); - tmp.assume_init() + + #[cfg(bootstrap)] + { + // We are calling the intrinsics directly to avoid function calls in the + // generated code as `intrinsics::copy_nonoverlapping` is a wrapper function. + extern "rust-intrinsic" { + #[rustc_const_stable(feature = "const_intrinsic_copy", since = "1.63.0")] + fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize); + } + + // `src` cannot overlap `tmp` because `tmp` was just allocated on + // the stack as a separate allocated object. + let mut tmp = MaybeUninit::<T>::uninit(); + copy_nonoverlapping(src, tmp.as_mut_ptr(), 1); + tmp.assume_init() + } + #[cfg(not(bootstrap))] + { + crate::intrinsics::read_via_copy(src) + } } } |