Auto merge of rust-lang#108483 - scottmcm:unify-bytewise-eq-traits, r=the8472

Merge two different equality specialization traits in `core`

Arrays and slices each had their own version of this, without a matching set of `impl`s.

Merge them into one (still-`pub(crate)`) `cmp::BytewiseEq` trait, so we can stop doing all these things twice.

And that means that the `[T]::eq` → `memcmp` specialization picks up a bunch of types where that previously only worked for arrays, so examples like <https://rust.godbolt.org/z/KjsG8MGGT> will use it now instead of emitting loops.

r? the8472

Author: bors

Diff for: library/core/src/array/equality.rs

@@ -1,6 +1,5 @@
+use crate::cmp::BytewiseEq;
 use crate::convert::TryInto;
-use crate::num::{NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, NonZeroIsize};
-use crate::num::{NonZeroU128, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize};
 
 #[stable(feature = "rust1", since = "1.0.0")]
 impl<A, B, const N: usize> PartialEq<[B; N]> for [A; N]
@@ -144,74 +143,14 @@ impl<T: PartialEq<Other>, Other, const N: usize> SpecArrayEq<Other, N> for T {
     }
 }
 
-impl<T: IsRawEqComparable<U>, U, const N: usize> SpecArrayEq<U, N> for T {
+impl<T: BytewiseEq<U>, U, const N: usize> SpecArrayEq<U, N> for T {
     fn spec_eq(a: &[T; N], b: &[U; N]) -> bool {
-        // SAFETY: This is why `IsRawEqComparable` is an `unsafe trait`.
-        unsafe {
-            let b = &*b.as_ptr().cast::<[T; N]>();
-            crate::intrinsics::raw_eq(a, b)
-        }
+        // SAFETY: Arrays are compared element-wise, and don't add any padding
+        // between elements, so when the elements are `BytewiseEq`, we can
+        // compare the entire array at once.
+        unsafe { crate::intrinsics::raw_eq(a, crate::mem::transmute(b)) }
     }
     fn spec_ne(a: &[T; N], b: &[U; N]) -> bool {
         !Self::spec_eq(a, b)
     }
 }
-
-/// `U` exists on here mostly because `min_specialization` didn't let me
-/// repeat the `T` type parameter in the above specialization, so instead
-/// the `T == U` constraint comes from the impls on this.
-/// # Safety
-/// - Neither `Self` nor `U` has any padding.
-/// - `Self` and `U` have the same layout.
-/// - `Self: PartialEq<U>` is byte-wise (this means no floats, among other things)
-#[rustc_specialization_trait]
-unsafe trait IsRawEqComparable<U>: PartialEq<U> {}
-
-macro_rules! is_raw_eq_comparable {
-    ($($t:ty),+ $(,)?) => {$(
-        unsafe impl IsRawEqComparable<$t> for $t {}
-    )+};
-}
-
-// SAFETY: All the ordinary integer types have no padding, and are not pointers.
-is_raw_eq_comparable!(u8, u16, u32, u64, u128, usize, i8, i16, i32, i64, i128, isize);
-
-// SAFETY: bool and char have *niches*, but no *padding* (and these are not pointer types), so this
-// is sound
-is_raw_eq_comparable!(bool, char);
-
-// SAFETY: Similarly, the non-zero types have a niche, but no undef and no pointers,
-// and they compare like their underlying numeric type.
-is_raw_eq_comparable!(
-    NonZeroU8,
-    NonZeroU16,
-    NonZeroU32,
-    NonZeroU64,
-    NonZeroU128,
-    NonZeroUsize,
-    NonZeroI8,
-    NonZeroI16,
-    NonZeroI32,
-    NonZeroI64,
-    NonZeroI128,
-    NonZeroIsize,
-);
-
-// SAFETY: The NonZero types have the "null" optimization guaranteed, and thus
-// are also safe to equality-compare bitwise inside an `Option`.
-// The way `PartialOrd` is defined for `Option` means that this wouldn't work
-// for `<` or `>` on the signed types, but since we only do `==` it's fine.
-is_raw_eq_comparable!(
-    Option<NonZeroU8>,
-    Option<NonZeroU16>,
-    Option<NonZeroU32>,
-    Option<NonZeroU64>,
-    Option<NonZeroU128>,
-    Option<NonZeroUsize>,
-    Option<NonZeroI8>,
-    Option<NonZeroI16>,
-    Option<NonZeroI32>,
-    Option<NonZeroI64>,
-    Option<NonZeroI128>,
-    Option<NonZeroIsize>,
-);

Diff for: library/core/src/cmp.rs

@@ -22,6 +22,9 @@
 
 #![stable(feature = "rust1", since = "1.0.0")]
 
+mod bytewise;
+pub(crate) use bytewise::BytewiseEq;
+
 use crate::marker::Destruct;
 
 use self::Ordering::*;

Diff for: library/core/src/cmp/bytewise.rs

@@ -0,0 +1,83 @@
+use crate::num::{NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, NonZeroIsize};
+use crate::num::{NonZeroU128, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize};
+
+/// Types where `==` & `!=` are equivalent to comparing their underlying bytes.
+///
+/// Importantly, this means no floating-point types, as those have different
+/// byte representations (like `-0` and `+0`) which compare as the same.
+/// Since byte arrays are `Eq`, that implies that these types are probably also
+/// `Eq`, but that's not technically required to use this trait.
+///
+/// `Rhs` is *de facto* always `Self`, but the separate parameter is important
+/// to avoid the `specializing impl repeats parameter` error when consuming this.
+///
+/// # Safety
+///
+/// - `Self` and `Rhs` have no padding.
+/// - `Self` and `Rhs` have the same layout (size and alignment).
+/// - Neither `Self` nor `Rhs` have provenance, so integer comparisons are correct.
+/// - `<Self as PartialEq<Rhs>>::{eq,ne}` are equivalent to comparing the bytes.
+#[rustc_specialization_trait]
+pub(crate) unsafe trait BytewiseEq<Rhs = Self>: PartialEq<Rhs> + Sized {}
+
+macro_rules! is_bytewise_comparable {
+    ($($t:ty),+ $(,)?) => {$(
+        unsafe impl BytewiseEq for $t {}
+    )+};
+}
+
+// SAFETY: All the ordinary integer types have no padding, and are not pointers.
+is_bytewise_comparable!(u8, u16, u32, u64, u128, usize, i8, i16, i32, i64, i128, isize);
+
+// SAFETY: These have *niches*, but no *padding* and no *provenance*,
+// so we can compare them directly.
+is_bytewise_comparable!(bool, char, super::Ordering);
+
+// SAFETY: Similarly, the non-zero types have a niche, but no undef and no pointers,
+// and they compare like their underlying numeric type.
+is_bytewise_comparable!(
+    NonZeroU8,
+    NonZeroU16,
+    NonZeroU32,
+    NonZeroU64,
+    NonZeroU128,
+    NonZeroUsize,
+    NonZeroI8,
+    NonZeroI16,
+    NonZeroI32,
+    NonZeroI64,
+    NonZeroI128,
+    NonZeroIsize,
+);
+
+// SAFETY: The NonZero types have the "null" optimization guaranteed, and thus
+// are also safe to equality-compare bitwise inside an `Option`.
+// The way `PartialOrd` is defined for `Option` means that this wouldn't work
+// for `<` or `>` on the signed types, but since we only do `==` it's fine.
+is_bytewise_comparable!(
+    Option<NonZeroU8>,
+    Option<NonZeroU16>,
+    Option<NonZeroU32>,
+    Option<NonZeroU64>,
+    Option<NonZeroU128>,
+    Option<NonZeroUsize>,
+    Option<NonZeroI8>,
+    Option<NonZeroI16>,
+    Option<NonZeroI32>,
+    Option<NonZeroI64>,
+    Option<NonZeroI128>,
+    Option<NonZeroIsize>,
+);
+
+macro_rules! is_bytewise_comparable_array_length {
+    ($($n:literal),+ $(,)?) => {$(
+        // SAFETY: Arrays have no padding between elements, so if the elements are
+        // `BytewiseEq`, then the whole array can be too.
+        unsafe impl<T: BytewiseEq<U>, U> BytewiseEq<[U; $n]> for [T; $n] {}
+    )+};
+}
+
+// Frustratingly, this can't be made const-generic as it gets
+//    error: specializing impl repeats parameter `N`
+// so just do it for a couple of plausibly-common ones.
+is_bytewise_comparable_array_length!(0, 1, 2, 3, 4, 6, 8, 12, 16, 24, 32, 48, 64);

Diff for: library/core/src/intrinsics.rs

@@ -2093,6 +2093,10 @@ extern "rust-intrinsic" {
     /// Above some backend-decided threshold this will emit calls to `memcmp`,
     /// like slice equality does, instead of causing massive code size.
     ///
+    /// Since this works by comparing the underlying bytes, the actual `T` is
+    /// not particularly important.  It will be used for its size and alignment,
+    /// but any validity restrictions will be ignored, not enforced.
+    ///
     /// # Safety
     ///
     /// It's UB to call this if any of the *bytes* in `*a` or `*b` are uninitialized or carry a

Diff for: library/core/src/slice/cmp.rs

@@ -1,6 +1,6 @@
 //! Comparison traits for `[T]`.
 
-use crate::cmp::{self, Ordering};
+use crate::cmp::{self, BytewiseEq, Ordering};
 use crate::ffi;
 use crate::mem;
 
@@ -77,7 +77,7 @@ where
 // Use memcmp for bytewise equality when the types allow
 impl<A, B> SlicePartialEq<B> for [A]
 where
-    A: BytewiseEquality<B>,
+    A: BytewiseEq<B>,
 {
     fn equal(&self, other: &[B]) -> bool {
         if self.len() != other.len() {
@@ -203,29 +203,6 @@ impl SliceOrd for u8 {
     }
 }
 
-// Hack to allow specializing on `Eq` even though `Eq` has a method.
-#[rustc_unsafe_specialization_marker]
-trait MarkerEq<T>: PartialEq<T> {}
-
-impl<T: Eq> MarkerEq<T> for T {}
-
-#[doc(hidden)]
-/// Trait implemented for types that can be compared for equality using
-/// their bytewise representation
-#[rustc_specialization_trait]
-trait BytewiseEquality<T>: MarkerEq<T> + Copy {}
-
-macro_rules! impl_marker_for {
-    ($traitname:ident, $($ty:ty)*) => {
-        $(
-            impl $traitname<$ty> for $ty { }
-        )*
-    }
-}
-
-impl_marker_for!(BytewiseEquality,
-                 u8 i8 u16 i16 u32 i32 u64 i64 u128 i128 usize isize char bool);
-
 pub(super) trait SliceContains: Sized {
     fn slice_contains(&self, x: &[Self]) -> bool;
 }

Diff for: tests/codegen/array-equality.rs

@@ -1,4 +1,4 @@
-// compile-flags: -O
+// compile-flags: -O -Z merge-functions=disabled
 // only-x86_64
 
 #![crate_type = "lib"]
@@ -43,6 +43,15 @@ pub fn array_eq_long(a: &[u16; 1234], b: &[u16; 1234]) -> bool {
     a == b
 }
 
+// CHECK-LABEL: @array_char_eq
+#[no_mangle]
+pub fn array_char_eq(a: [char; 2], b: [char; 2]) -> bool {
+    // CHECK-NEXT: start:
+    // CHECK-NEXT: %[[EQ:.+]] = icmp eq i64 %0, %1
+    // CHECK-NEXT: ret i1 %[[EQ]]
+    a == b
+}
+
 // CHECK-LABEL: @array_eq_zero_short(i48
 #[no_mangle]
 pub fn array_eq_zero_short(x: [u16; 3]) -> bool {
@@ -52,6 +61,25 @@ pub fn array_eq_zero_short(x: [u16; 3]) -> bool {
     x == [0; 3]
 }
 
+// CHECK-LABEL: @array_eq_none_short(i40
+#[no_mangle]
+pub fn array_eq_none_short(x: [Option<std::num::NonZeroU8>; 5]) -> bool {
+    // CHECK-NEXT: start:
+    // CHECK-NEXT: %[[EQ:.+]] = icmp eq i40 %0, 0
+    // CHECK-NEXT: ret i1 %[[EQ]]
+    x == [None; 5]
+}
+
+// CHECK-LABEL: @array_eq_zero_nested(
+#[no_mangle]
+pub fn array_eq_zero_nested(x: [[u8; 3]; 3]) -> bool {
+    // CHECK: %[[VAL:.+]] = load i72
+    // CHECK-SAME: align 1
+    // CHECK: %[[EQ:.+]] = icmp eq i72 %[[VAL]], 0
+    // CHECK: ret i1 %[[EQ]]
+    x == [[0; 3]; 3]
+}
+
 // CHECK-LABEL: @array_eq_zero_mid(
 #[no_mangle]
 pub fn array_eq_zero_mid(x: [u16; 8]) -> bool {

Diff for: tests/codegen/slice-ref-equality.rs

@@ -1,7 +1,10 @@
-// compile-flags: -C opt-level=3 -Zmerge-functions=disabled
+// compile-flags: -O -Zmerge-functions=disabled
+// ignore-debug (the extra assertions get in the way)
 
 #![crate_type = "lib"]
 
+use std::num::{NonZeroI16, NonZeroU32};
+
 // #71602 reported a simple array comparison just generating a loop.
 // This was originally fixed by ensuring it generates a single bcmp,
 // but we now generate it as a load+icmp instead. `is_zero_slice` was
@@ -36,3 +39,54 @@ pub fn is_zero_array(data: &[u8; 4]) -> bool {
     // CHECK-NEXT: ret i1 %[[EQ]]
     *data == [0; 4]
 }
+
+// The following test the extra specializations to make sure that slice
+// equality for non-byte types also just emit a `bcmp`, not a loop.
+
+// CHECK-LABEL: @eq_slice_of_nested_u8(
+// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
+// CHECK-SAME: [[USIZE]] noundef %3
+#[no_mangle]
+fn eq_slice_of_nested_u8(x: &[[u8; 3]], y: &[[u8; 3]]) -> bool {
+    // CHECK: icmp eq [[USIZE]] %1, %3
+    // CHECK: %[[BYTES:.+]] = mul nsw [[USIZE]] %1, 3
+    // CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}({{i8\*|ptr}}
+    // CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
+    x == y
+}
+
+// CHECK-LABEL: @eq_slice_of_i32(
+// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
+// CHECK-SAME: [[USIZE]] noundef %3
+#[no_mangle]
+fn eq_slice_of_i32(x: &[i32], y: &[i32]) -> bool {
+    // CHECK: icmp eq [[USIZE]] %1, %3
+    // CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %1, 2
+    // CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}({{i32\*|ptr}}
+    // CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
+    x == y
+}
+
+// CHECK-LABEL: @eq_slice_of_nonzero(
+// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
+// CHECK-SAME: [[USIZE]] noundef %3
+#[no_mangle]
+fn eq_slice_of_nonzero(x: &[NonZeroU32], y: &[NonZeroU32]) -> bool {
+    // CHECK: icmp eq [[USIZE]] %1, %3
+    // CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %1, 2
+    // CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}({{i32\*|ptr}}
+    // CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
+    x == y
+}
+
+// CHECK-LABEL: @eq_slice_of_option_of_nonzero(
+// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
+// CHECK-SAME: [[USIZE]] noundef %3
+#[no_mangle]
+fn eq_slice_of_option_of_nonzero(x: &[Option<NonZeroI16>], y: &[Option<NonZeroI16>]) -> bool {
+    // CHECK: icmp eq [[USIZE]] %1, %3
+    // CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %1, 1
+    // CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}({{i16\*|ptr}}
+    // CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
+    x == y
+}