Skip to content

Commit 0b4ba4c

Browse files
committed
Auto merge of #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
2 parents f77bfb7 + 44eec1d commit 0b4ba4c

File tree

7 files changed

+182
-94
lines changed

7 files changed

+182
-94
lines changed

library/core/src/array/equality.rs

+6-67
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1+
use crate::cmp::BytewiseEq;
12
use crate::convert::TryInto;
2-
use crate::num::{NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, NonZeroIsize};
3-
use crate::num::{NonZeroU128, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize};
43

54
#[stable(feature = "rust1", since = "1.0.0")]
65
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 {
144143
}
145144
}
146145

147-
impl<T: IsRawEqComparable<U>, U, const N: usize> SpecArrayEq<U, N> for T {
146+
impl<T: BytewiseEq<U>, U, const N: usize> SpecArrayEq<U, N> for T {
148147
fn spec_eq(a: &[T; N], b: &[U; N]) -> bool {
149-
// SAFETY: This is why `IsRawEqComparable` is an `unsafe trait`.
150-
unsafe {
151-
let b = &*b.as_ptr().cast::<[T; N]>();
152-
crate::intrinsics::raw_eq(a, b)
153-
}
148+
// SAFETY: Arrays are compared element-wise, and don't add any padding
149+
// between elements, so when the elements are `BytewiseEq`, we can
150+
// compare the entire array at once.
151+
unsafe { crate::intrinsics::raw_eq(a, crate::mem::transmute(b)) }
154152
}
155153
fn spec_ne(a: &[T; N], b: &[U; N]) -> bool {
156154
!Self::spec_eq(a, b)
157155
}
158156
}
159-
160-
/// `U` exists on here mostly because `min_specialization` didn't let me
161-
/// repeat the `T` type parameter in the above specialization, so instead
162-
/// the `T == U` constraint comes from the impls on this.
163-
/// # Safety
164-
/// - Neither `Self` nor `U` has any padding.
165-
/// - `Self` and `U` have the same layout.
166-
/// - `Self: PartialEq<U>` is byte-wise (this means no floats, among other things)
167-
#[rustc_specialization_trait]
168-
unsafe trait IsRawEqComparable<U>: PartialEq<U> {}
169-
170-
macro_rules! is_raw_eq_comparable {
171-
($($t:ty),+ $(,)?) => {$(
172-
unsafe impl IsRawEqComparable<$t> for $t {}
173-
)+};
174-
}
175-
176-
// SAFETY: All the ordinary integer types have no padding, and are not pointers.
177-
is_raw_eq_comparable!(u8, u16, u32, u64, u128, usize, i8, i16, i32, i64, i128, isize);
178-
179-
// SAFETY: bool and char have *niches*, but no *padding* (and these are not pointer types), so this
180-
// is sound
181-
is_raw_eq_comparable!(bool, char);
182-
183-
// SAFETY: Similarly, the non-zero types have a niche, but no undef and no pointers,
184-
// and they compare like their underlying numeric type.
185-
is_raw_eq_comparable!(
186-
NonZeroU8,
187-
NonZeroU16,
188-
NonZeroU32,
189-
NonZeroU64,
190-
NonZeroU128,
191-
NonZeroUsize,
192-
NonZeroI8,
193-
NonZeroI16,
194-
NonZeroI32,
195-
NonZeroI64,
196-
NonZeroI128,
197-
NonZeroIsize,
198-
);
199-
200-
// SAFETY: The NonZero types have the "null" optimization guaranteed, and thus
201-
// are also safe to equality-compare bitwise inside an `Option`.
202-
// The way `PartialOrd` is defined for `Option` means that this wouldn't work
203-
// for `<` or `>` on the signed types, but since we only do `==` it's fine.
204-
is_raw_eq_comparable!(
205-
Option<NonZeroU8>,
206-
Option<NonZeroU16>,
207-
Option<NonZeroU32>,
208-
Option<NonZeroU64>,
209-
Option<NonZeroU128>,
210-
Option<NonZeroUsize>,
211-
Option<NonZeroI8>,
212-
Option<NonZeroI16>,
213-
Option<NonZeroI32>,
214-
Option<NonZeroI64>,
215-
Option<NonZeroI128>,
216-
Option<NonZeroIsize>,
217-
);

library/core/src/cmp.rs

+3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
2323
#![stable(feature = "rust1", since = "1.0.0")]
2424

25+
mod bytewise;
26+
pub(crate) use bytewise::BytewiseEq;
27+
2528
use crate::marker::Destruct;
2629

2730
use self::Ordering::*;

library/core/src/cmp/bytewise.rs

+83
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
use crate::num::{NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, NonZeroIsize};
2+
use crate::num::{NonZeroU128, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize};
3+
4+
/// Types where `==` & `!=` are equivalent to comparing their underlying bytes.
5+
///
6+
/// Importantly, this means no floating-point types, as those have different
7+
/// byte representations (like `-0` and `+0`) which compare as the same.
8+
/// Since byte arrays are `Eq`, that implies that these types are probably also
9+
/// `Eq`, but that's not technically required to use this trait.
10+
///
11+
/// `Rhs` is *de facto* always `Self`, but the separate parameter is important
12+
/// to avoid the `specializing impl repeats parameter` error when consuming this.
13+
///
14+
/// # Safety
15+
///
16+
/// - `Self` and `Rhs` have no padding.
17+
/// - `Self` and `Rhs` have the same layout (size and alignment).
18+
/// - Neither `Self` nor `Rhs` have provenance, so integer comparisons are correct.
19+
/// - `<Self as PartialEq<Rhs>>::{eq,ne}` are equivalent to comparing the bytes.
20+
#[rustc_specialization_trait]
21+
pub(crate) unsafe trait BytewiseEq<Rhs = Self>: PartialEq<Rhs> + Sized {}
22+
23+
macro_rules! is_bytewise_comparable {
24+
($($t:ty),+ $(,)?) => {$(
25+
unsafe impl BytewiseEq for $t {}
26+
)+};
27+
}
28+
29+
// SAFETY: All the ordinary integer types have no padding, and are not pointers.
30+
is_bytewise_comparable!(u8, u16, u32, u64, u128, usize, i8, i16, i32, i64, i128, isize);
31+
32+
// SAFETY: These have *niches*, but no *padding* and no *provenance*,
33+
// so we can compare them directly.
34+
is_bytewise_comparable!(bool, char, super::Ordering);
35+
36+
// SAFETY: Similarly, the non-zero types have a niche, but no undef and no pointers,
37+
// and they compare like their underlying numeric type.
38+
is_bytewise_comparable!(
39+
NonZeroU8,
40+
NonZeroU16,
41+
NonZeroU32,
42+
NonZeroU64,
43+
NonZeroU128,
44+
NonZeroUsize,
45+
NonZeroI8,
46+
NonZeroI16,
47+
NonZeroI32,
48+
NonZeroI64,
49+
NonZeroI128,
50+
NonZeroIsize,
51+
);
52+
53+
// SAFETY: The NonZero types have the "null" optimization guaranteed, and thus
54+
// are also safe to equality-compare bitwise inside an `Option`.
55+
// The way `PartialOrd` is defined for `Option` means that this wouldn't work
56+
// for `<` or `>` on the signed types, but since we only do `==` it's fine.
57+
is_bytewise_comparable!(
58+
Option<NonZeroU8>,
59+
Option<NonZeroU16>,
60+
Option<NonZeroU32>,
61+
Option<NonZeroU64>,
62+
Option<NonZeroU128>,
63+
Option<NonZeroUsize>,
64+
Option<NonZeroI8>,
65+
Option<NonZeroI16>,
66+
Option<NonZeroI32>,
67+
Option<NonZeroI64>,
68+
Option<NonZeroI128>,
69+
Option<NonZeroIsize>,
70+
);
71+
72+
macro_rules! is_bytewise_comparable_array_length {
73+
($($n:literal),+ $(,)?) => {$(
74+
// SAFETY: Arrays have no padding between elements, so if the elements are
75+
// `BytewiseEq`, then the whole array can be too.
76+
unsafe impl<T: BytewiseEq<U>, U> BytewiseEq<[U; $n]> for [T; $n] {}
77+
)+};
78+
}
79+
80+
// Frustratingly, this can't be made const-generic as it gets
81+
// error: specializing impl repeats parameter `N`
82+
// so just do it for a couple of plausibly-common ones.
83+
is_bytewise_comparable_array_length!(0, 1, 2, 3, 4, 6, 8, 12, 16, 24, 32, 48, 64);

library/core/src/intrinsics.rs

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

library/core/src/slice/cmp.rs

+2-25
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Comparison traits for `[T]`.
22
3-
use crate::cmp::{self, Ordering};
3+
use crate::cmp::{self, BytewiseEq, Ordering};
44
use crate::ffi;
55
use crate::mem;
66

@@ -77,7 +77,7 @@ where
7777
// Use memcmp for bytewise equality when the types allow
7878
impl<A, B> SlicePartialEq<B> for [A]
7979
where
80-
A: BytewiseEquality<B>,
80+
A: BytewiseEq<B>,
8181
{
8282
fn equal(&self, other: &[B]) -> bool {
8383
if self.len() != other.len() {
@@ -203,29 +203,6 @@ impl SliceOrd for u8 {
203203
}
204204
}
205205

206-
// Hack to allow specializing on `Eq` even though `Eq` has a method.
207-
#[rustc_unsafe_specialization_marker]
208-
trait MarkerEq<T>: PartialEq<T> {}
209-
210-
impl<T: Eq> MarkerEq<T> for T {}
211-
212-
#[doc(hidden)]
213-
/// Trait implemented for types that can be compared for equality using
214-
/// their bytewise representation
215-
#[rustc_specialization_trait]
216-
trait BytewiseEquality<T>: MarkerEq<T> + Copy {}
217-
218-
macro_rules! impl_marker_for {
219-
($traitname:ident, $($ty:ty)*) => {
220-
$(
221-
impl $traitname<$ty> for $ty { }
222-
)*
223-
}
224-
}
225-
226-
impl_marker_for!(BytewiseEquality,
227-
u8 i8 u16 i16 u32 i32 u64 i64 u128 i128 usize isize char bool);
228-
229206
pub(super) trait SliceContains: Sized {
230207
fn slice_contains(&self, x: &[Self]) -> bool;
231208
}

tests/codegen/array-equality.rs

+29-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// compile-flags: -O
1+
// compile-flags: -O -Z merge-functions=disabled
22
// only-x86_64
33

44
#![crate_type = "lib"]
@@ -43,6 +43,15 @@ pub fn array_eq_long(a: &[u16; 1234], b: &[u16; 1234]) -> bool {
4343
a == b
4444
}
4545

46+
// CHECK-LABEL: @array_char_eq
47+
#[no_mangle]
48+
pub fn array_char_eq(a: [char; 2], b: [char; 2]) -> bool {
49+
// CHECK-NEXT: start:
50+
// CHECK-NEXT: %[[EQ:.+]] = icmp eq i64 %0, %1
51+
// CHECK-NEXT: ret i1 %[[EQ]]
52+
a == b
53+
}
54+
4655
// CHECK-LABEL: @array_eq_zero_short(i48
4756
#[no_mangle]
4857
pub fn array_eq_zero_short(x: [u16; 3]) -> bool {
@@ -52,6 +61,25 @@ pub fn array_eq_zero_short(x: [u16; 3]) -> bool {
5261
x == [0; 3]
5362
}
5463

64+
// CHECK-LABEL: @array_eq_none_short(i40
65+
#[no_mangle]
66+
pub fn array_eq_none_short(x: [Option<std::num::NonZeroU8>; 5]) -> bool {
67+
// CHECK-NEXT: start:
68+
// CHECK-NEXT: %[[EQ:.+]] = icmp eq i40 %0, 0
69+
// CHECK-NEXT: ret i1 %[[EQ]]
70+
x == [None; 5]
71+
}
72+
73+
// CHECK-LABEL: @array_eq_zero_nested(
74+
#[no_mangle]
75+
pub fn array_eq_zero_nested(x: [[u8; 3]; 3]) -> bool {
76+
// CHECK: %[[VAL:.+]] = load i72
77+
// CHECK-SAME: align 1
78+
// CHECK: %[[EQ:.+]] = icmp eq i72 %[[VAL]], 0
79+
// CHECK: ret i1 %[[EQ]]
80+
x == [[0; 3]; 3]
81+
}
82+
5583
// CHECK-LABEL: @array_eq_zero_mid(
5684
#[no_mangle]
5785
pub fn array_eq_zero_mid(x: [u16; 8]) -> bool {

tests/codegen/slice-ref-equality.rs

+55-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1-
// compile-flags: -C opt-level=3 -Zmerge-functions=disabled
1+
// compile-flags: -O -Zmerge-functions=disabled
2+
// ignore-debug (the extra assertions get in the way)
23

34
#![crate_type = "lib"]
45

6+
use std::num::{NonZeroI16, NonZeroU32};
7+
58
// #71602 reported a simple array comparison just generating a loop.
69
// This was originally fixed by ensuring it generates a single bcmp,
710
// 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 {
3639
// CHECK-NEXT: ret i1 %[[EQ]]
3740
*data == [0; 4]
3841
}
42+
43+
// The following test the extra specializations to make sure that slice
44+
// equality for non-byte types also just emit a `bcmp`, not a loop.
45+
46+
// CHECK-LABEL: @eq_slice_of_nested_u8(
47+
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
48+
// CHECK-SAME: [[USIZE]] noundef %3
49+
#[no_mangle]
50+
fn eq_slice_of_nested_u8(x: &[[u8; 3]], y: &[[u8; 3]]) -> bool {
51+
// CHECK: icmp eq [[USIZE]] %1, %3
52+
// CHECK: %[[BYTES:.+]] = mul nsw [[USIZE]] %1, 3
53+
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}({{i8\*|ptr}}
54+
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
55+
x == y
56+
}
57+
58+
// CHECK-LABEL: @eq_slice_of_i32(
59+
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
60+
// CHECK-SAME: [[USIZE]] noundef %3
61+
#[no_mangle]
62+
fn eq_slice_of_i32(x: &[i32], y: &[i32]) -> bool {
63+
// CHECK: icmp eq [[USIZE]] %1, %3
64+
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %1, 2
65+
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}({{i32\*|ptr}}
66+
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
67+
x == y
68+
}
69+
70+
// CHECK-LABEL: @eq_slice_of_nonzero(
71+
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
72+
// CHECK-SAME: [[USIZE]] noundef %3
73+
#[no_mangle]
74+
fn eq_slice_of_nonzero(x: &[NonZeroU32], y: &[NonZeroU32]) -> bool {
75+
// CHECK: icmp eq [[USIZE]] %1, %3
76+
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %1, 2
77+
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}({{i32\*|ptr}}
78+
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
79+
x == y
80+
}
81+
82+
// CHECK-LABEL: @eq_slice_of_option_of_nonzero(
83+
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
84+
// CHECK-SAME: [[USIZE]] noundef %3
85+
#[no_mangle]
86+
fn eq_slice_of_option_of_nonzero(x: &[Option<NonZeroI16>], y: &[Option<NonZeroI16>]) -> bool {
87+
// CHECK: icmp eq [[USIZE]] %1, %3
88+
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %1, 1
89+
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}({{i16\*|ptr}}
90+
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
91+
x == y
92+
}

0 commit comments

Comments
 (0)