Auto merge of rust-lang#122582 - scottmcm:swap-intrinsic-v2, r=<try>

Let codegen decide when to `mem::swap` with immediates

Making `libcore` decide this is silly; the backend has so much better information about when it's a good idea.

Thus this PR introduces a new `typed_swap` intrinsic with a fallback body, and replaces that fallback implementation when swapping immediates or scalar pairs.

r? oli-obk

Replaces rust-lang#111744, and means we'll never need more libs PRs like rust-lang#111803 or rust-lang#107140

Author: bors

compiler/rustc_codegen_ssa/src/mir/intrinsic.rs

@@ -76,6 +76,22 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         let name = bx.tcx().item_name(def_id);
         let name_str = name.as_str();
 
+        // If we're swapping something that's *not* an `OperandValue::Ref`,
+        // then we can do it directly and avoid the alloca.
+        // Otherwise, we'll let the fallback MIR body take care of it.
+        if let sym::typed_swap = name {
+            let pointee_ty = fn_args.type_at(0);
+            let pointee_layout = bx.layout_of(pointee_ty);
+            if !bx.is_backend_ref(pointee_layout) {
+                let x_place = PlaceRef::new_sized(args[0].immediate(), pointee_layout);
+                let y_place = PlaceRef::new_sized(args[1].immediate(), pointee_layout);
+                let temp = bx.load_operand(x_place);
+                bx.typed_place_copy(x_place, y_place);
+                temp.val.store(bx, y_place);
+                return Ok(());
+            }
+        }
+
         let llret_ty = bx.backend_type(bx.layout_of(ret_ty));
         let result = PlaceRef::new_sized(llresult, fn_abi.ret.layout);
 

compiler/rustc_codegen_ssa/src/traits/builder.rs

@@ -1,5 +1,6 @@
 use super::abi::AbiBuilderMethods;
 use super::asm::AsmBuilderMethods;
+use super::consts::ConstMethods;
 use super::coverageinfo::CoverageInfoBuilderMethods;
 use super::debuginfo::DebugInfoBuilderMethods;
 use super::intrinsic::IntrinsicCallMethods;
@@ -267,6 +268,26 @@ pub trait BuilderMethods<'a, 'tcx>:
         flags: MemFlags,
     );
 
+    /// *Typed* copy for non-overlapping places.
+    ///
+    /// Has a default implementation in terms of `memcpy`, but specific backends
+    /// can override to do something smarter if possible.
+    ///
+    /// (For example, typed load-stores with alias metadata.)
+    fn typed_place_copy(
+        &mut self,
+        dst: PlaceRef<'tcx, Self::Value>,
+        src: PlaceRef<'tcx, Self::Value>,
+    ) {
+        debug_assert!(src.llextra.is_none());
+        debug_assert!(dst.llextra.is_none());
+        debug_assert_eq!(dst.layout.size, src.layout.size);
+        if !dst.layout.is_zst() {
+            let bytes = self.const_usize(dst.layout.size.bytes());
+            self.memcpy(dst.llval, dst.align, src.llval, src.align, bytes, MemFlags::empty());
+        }
+    }
+
     fn select(
         &mut self,
         cond: Self::Value,

compiler/rustc_codegen_ssa/src/traits/type_.rs

@@ -120,6 +120,20 @@ pub trait LayoutTypeMethods<'tcx>: Backend<'tcx> {
         immediate: bool,
     ) -> Self::Type;
 
+    /// A type that produces an [`OperandValue::Ref`] when loaded.
+    ///
+    /// AKA one that's not a ZST, not `is_backend_immediate`, and
+    /// not `is_backend_scalar_pair`. For such a type, a
+    /// [`load_operand`] doesn't actually `load` anything.
+    ///
+    /// [`OperandValue::Ref`]: crate::mir::operand::OperandValue::Ref
+    /// [`load_operand`]: super::BuilderMethods::load_operand
+    fn is_backend_ref(&self, layout: TyAndLayout<'tcx>) -> bool {
+        !(layout.is_zst()
+            || self.is_backend_immediate(layout)
+            || self.is_backend_scalar_pair(layout))
+    }
+
     /// A type that can be used in a [`super::BuilderMethods::load`] +
     /// [`super::BuilderMethods::store`] pair to implement a *typed* copy,
     /// such as a MIR `*_0 = *_1`.

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -21,8 +21,8 @@ use rustc_span::symbol::{sym, Symbol};
 use rustc_target::abi::Size;
 
 use super::{
-    util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, MPlaceTy, Machine, OpTy,
-    Pointer,
+    memory::MemoryKind, util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx,
+    MPlaceTy, Machine, OpTy, Pointer,
 };
 
 use crate::fluent_generated as fluent;
@@ -415,6 +415,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 let result = self.raw_eq_intrinsic(&args[0], &args[1])?;
                 self.write_scalar(result, dest)?;
             }
+            sym::typed_swap => {
+                self.typed_swap_intrinsic(&args[0], &args[1])?;
+            }
 
             sym::vtable_size => {
                 let ptr = self.read_pointer(&args[0])?;
@@ -608,6 +611,24 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         self.mem_copy(src, dst, size, nonoverlapping)
     }
 
+    /// Does a *typed* swap of `*left` and `*right`.
+    pub(crate) fn typed_swap_intrinsic(
+        &mut self,
+        left: &OpTy<'tcx, <M as Machine<'mir, 'tcx>>::Provenance>,
+        right: &OpTy<'tcx, <M as Machine<'mir, 'tcx>>::Provenance>,
+    ) -> InterpResult<'tcx> {
+        let left = self.deref_pointer(left)?;
+        let right = self.deref_pointer(right)?;
+        debug_assert_eq!(left.layout, right.layout);
+        let kind = MemoryKind::Stack;
+        let temp = self.allocate(left.layout, kind)?;
+        self.copy_op(&left, &temp)?;
+        self.copy_op(&right, &left)?;
+        self.copy_op(&temp, &right)?;
+        self.deallocate_ptr(temp.ptr(), None, kind)?;
+        Ok(())
+    }
+
     pub(crate) fn write_bytes_intrinsic(
         &mut self,
         dst: &OpTy<'tcx, <M as Machine<'mir, 'tcx>>::Provenance>,

compiler/rustc_hir_analysis/src/check/intrinsic.rs

@@ -500,6 +500,8 @@ pub fn check_intrinsic_type(
                 (1, 0, vec![Ty::new_mut_ptr(tcx, param(0)), param(0)], Ty::new_unit(tcx))
             }
 
+            sym::typed_swap => (1, 1, vec![Ty::new_mut_ptr(tcx, param(0)); 2], Ty::new_unit(tcx)),
+
             sym::discriminant_value => {
                 let assoc_items = tcx.associated_item_def_ids(
                     tcx.require_lang_item(hir::LangItem::DiscriminantKind, None),

compiler/rustc_span/src/symbol.rs

@@ -1835,6 +1835,7 @@ symbols! {
         type_macros,
         type_name,
         type_privacy_lints,
+        typed_swap,
         u128,
         u128_legacy_const_max,
         u128_legacy_const_min,

library/core/src/mem/mod.rs

@@ -726,31 +726,19 @@ pub unsafe fn uninitialized<T>() -> T {
 #[rustc_const_unstable(feature = "const_swap", issue = "83163")]
 #[rustc_diagnostic_item = "mem_swap"]
 pub const fn swap<T>(x: &mut T, y: &mut T) {
-    // NOTE(eddyb) SPIR-V's Logical addressing model doesn't allow for arbitrary
-    // reinterpretation of values as (chunkable) byte arrays, and the loop in the
-    // block optimization in `swap_slice` is hard to rewrite back
-    // into the (unoptimized) direct swapping implementation, so we disable it.
-    #[cfg(not(any(target_arch = "spirv")))]
-    {
-        // For types that are larger multiples of their alignment, the simple way
-        // tends to copy the whole thing to stack rather than doing it one part
-        // at a time, so instead treat them as one-element slices and piggy-back
-        // the slice optimizations that will split up the swaps.
-        if const { size_of::<T>() / align_of::<T>() > 2 } {
-            // SAFETY: exclusive references always point to one non-overlapping
-            // element and are non-null and properly aligned.
-            return unsafe { ptr::swap_nonoverlapping(x, y, 1) };
+    cfg_if! {
+        // NOTE(eddyb) SPIR-V's Logical addressing model doesn't allow for arbitrary
+        // reinterpretation of values as (chunkable) byte arrays, and the loop in the
+        // block optimization in `ptr::swap_nonoverlapping` is hard to rewrite back
+        // into the (unoptimized) direct swapping implementation, so we disable it.
+        if #[cfg(any(target_arch = "spirv"))] {
+            swap_simple(x, y)
+        } else {
+            // SAFETY: `&mut` guarantees these are typed readable and writable
+            // as well as non-overlapping.
+            unsafe { ptr::typed_swap(x, y) }
         }
     }
-
-    // If a scalar consists of just a small number of alignment units, let
-    // the codegen just swap those pieces directly, as it's likely just a
-    // few instructions and anything else is probably overcomplicated.
-    //
-    // Most importantly, this covers primitives and simd types that tend to
-    // have size=align where doing anything else can be a pessimization.
-    // (This will also be used for ZSTs, though any solution works for them.)
-    swap_simple(x, y);
 }
 
 /// Same as [`swap`] semantically, but always uses the simple implementation.

library/core/src/ptr/mod.rs

@@ -954,6 +954,23 @@ pub const unsafe fn swap<T>(x: *mut T, y: *mut T) {
     }
 }
 
+/// Non-overlapping *typed* swap of a single value.
+///
+/// The codegen backends will replace this with a better implementation when
+/// `T` is a simple type that can be an immediate.
+///
+/// # Safety
+///
+/// `x` and `y` are readable and writable as `T`, and non-overlapping.
+#[rustc_nounwind]
+#[inline]
+#[cfg_attr(not(bootstrap), rustc_intrinsic)]
+pub(crate) const unsafe fn typed_swap<T>(x: *mut T, y: *mut T) {
+    // SAFETY: The caller provided single non-overlapping items behind
+    // pointers, so swapping them with `count: 1` is fine.
+    unsafe { swap_nonoverlapping(x, y, 1) };
+}
+
 /// Swaps `count * size_of::<T>()` bytes between the two regions of memory
 /// beginning at `x` and `y`. The two regions must *not* overlap.
 ///

src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-array.rs

@@ -0,0 +1,22 @@
+#![feature(rustc_attrs)]
+
+use std::ptr::addr_of_mut;
+
+#[rustc_intrinsic]
+const unsafe fn typed_swap<T, const WHATEVER: usize>(_: *mut T, _: *mut T) {
+    unreachable!()
+}
+
+fn invalid_array() {
+    let mut a = [1_u8; 100];
+    let mut b = [2_u8; 100];
+    unsafe {
+        let a = &mut *addr_of_mut!(a).cast::<[bool; 100]>();
+        let b = &mut *addr_of_mut!(b).cast::<[bool; 100]>();
+        typed_swap::<_, 0>(a, b); //~ERROR: constructing invalid value
+    }
+}
+
+fn main() {
+    invalid_array();
+}

src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-array.stderr

@@ -0,0 +1,20 @@
+error: Undefined Behavior: constructing invalid value at [0]: encountered 0x02, but expected a boolean
+  --> $DIR/typed-swap-invalid-array.rs:LL:CC
+   |
+LL |         typed_swap::<_, 0>(a, b);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at [0]: encountered 0x02, but expected a boolean
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `invalid_array` at $DIR/typed-swap-invalid-array.rs:LL:CC
+note: inside `main`
+  --> $DIR/typed-swap-invalid-array.rs:LL:CC
+   |
+LL |     invalid_array();
+   |     ^^^^^^^^^^^^^^^
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.rs

@@ -0,0 +1,22 @@
+#![feature(rustc_attrs)]
+
+use std::ptr::addr_of_mut;
+
+#[rustc_intrinsic]
+const unsafe fn typed_swap<T, const WHATEVER: usize>(_: *mut T, _: *mut T) {
+    unreachable!()
+}
+
+fn invalid_scalar() {
+    let mut a = 1_u8;
+    let mut b = 2_u8;
+    unsafe {
+        let a = &mut *addr_of_mut!(a).cast::<bool>();
+        let b = &mut *addr_of_mut!(b).cast::<bool>();
+        typed_swap::<_, 0>(a, b); //~ERROR: constructing invalid value
+    }
+}
+
+fn main() {
+    invalid_scalar();
+}

src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-scalar.stderr

@@ -0,0 +1,20 @@
+error: Undefined Behavior: constructing invalid value: encountered 0x02, but expected a boolean
+  --> $DIR/typed-swap-invalid-scalar.rs:LL:CC
+   |
+LL |         typed_swap::<_, 0>(a, b);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0x02, but expected a boolean
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `invalid_scalar` at $DIR/typed-swap-invalid-scalar.rs:LL:CC
+note: inside `main`
+  --> $DIR/typed-swap-invalid-scalar.rs:LL:CC
+   |
+LL |     invalid_scalar();
+   |     ^^^^^^^^^^^^^^^^
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

tests/assembly/x86_64-typed-swap.rs

@@ -0,0 +1,50 @@
+//@ only-x86_64
+//@ assembly-output: emit-asm
+//@ compile-flags: --crate-type=lib -O
+
+use std::arch::x86_64::__m128;
+use std::mem::swap;
+
+// CHECK-LABEL: swap_i32:
+#[no_mangle]
+pub fn swap_i32(x: &mut i32, y: &mut i32) {
+    // CHECK: movl (%[[ARG1:.+]]), %[[T1:.+]]
+    // CHECK: movl (%[[ARG2:.+]]), %[[T2:.+]]
+    // CHECK: movl %[[T2]], (%[[ARG1]])
+    // CHECK: movl %[[T1]], (%[[ARG2]])
+    // CHECK: retq
+    swap(x, y)
+}
+
+// CHECK-LABEL: swap_pair:
+#[no_mangle]
+pub fn swap_pair(x: &mut (i32, u32), y: &mut (i32, u32)) {
+    // CHECK: movq (%[[ARG1]]), %[[T1:.+]]
+    // CHECK: movq (%[[ARG2]]), %[[T2:.+]]
+    // CHECK: movq %[[T2]], (%[[ARG1]])
+    // CHECK: movq %[[T1]], (%[[ARG2]])
+    // CHECK: retq
+    swap(x, y)
+}
+
+// CHECK-LABEL: swap_str:
+#[no_mangle]
+pub fn swap_str<'a>(x: &mut &'a str, y: &mut &'a str) {
+    // CHECK: movups (%[[ARG1]]), %[[T1:xmm.]]
+    // CHECK: movups (%[[ARG2]]), %[[T2:xmm.]]
+    // CHECK: movups %[[T2]], (%[[ARG1]])
+    // CHECK: movups %[[T1]], (%[[ARG2]])
+    // CHECK: retq
+    swap(x, y)
+}
+
+// CHECK-LABEL: swap_simd:
+#[no_mangle]
+pub fn swap_simd(x: &mut __m128, y: &mut __m128) {
+    // CHECK: movaps (%[[ARG1]]), %[[T1:xmm.]]
+    // CHECK: movaps (%[[ARG2]]), %[[T2:xmm.]]
+    // CHECK: movaps %[[T2]], (%[[ARG1]])
+    // CHECK: movaps %[[T1]], (%[[ARG2]])
+    // CHECK: retq
+    swap(x, y)
+}