Skip to content

Commit

Permalink
Auto merge of #91838 - scottmcm:array-slice-eq-via-arrays-not-slices,…
Browse files Browse the repository at this point in the history
… r=dtolnay

Do array-slice equality via array equality, rather than always via slices

~~Draft because it needs a rebase after #91766 eventually gets through bors.~~

This enables the optimizations from #85828 to be used for array-to-slice comparisons too, not just array-to-array.

For example, <https://play.rust-lang.org/?version=nightly&mode=release&edition=2021&gist=5f9ba69b3d5825a782f897c830d3a6aa>
```rust
pub fn demo(x: &[u8], y: [u8; 4]) -> bool {
    *x == y
}
```
Currently writes the array to stack for no reason:
```nasm
	sub	rsp, 4
	mov	dword ptr [rsp], edx
	cmp	rsi, 4
	jne	.LBB0_1
	mov	eax, dword ptr [rdi]
	cmp	eax, dword ptr [rsp]
	sete	al
	add	rsp, 4
	ret

.LBB0_1:
	xor	eax, eax
	add	rsp, 4
	ret
```
Whereas with the change in this PR it just compares it directly:
```nasm
	cmp	rsi, 4
	jne	.LBB1_1
	cmp	dword ptr [rdi], edx
	sete	al
	ret

.LBB1_1:
	xor	eax, eax
	ret
```
  • Loading branch information
bors committed Dec 17, 2021
2 parents 34dc0d0 + a0b9690 commit 7abab1e
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 15 deletions.
41 changes: 29 additions & 12 deletions library/core/src/array/equality.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::convert::TryInto;
use crate::num::{NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, NonZeroIsize};
use crate::num::{NonZeroU128, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize};

Expand All @@ -23,11 +24,19 @@ where
{
#[inline]
fn eq(&self, other: &[B]) -> bool {
self[..] == other[..]
let b: Result<&[B; N], _> = other.try_into();
match b {
Ok(b) => *self == *b,
Err(_) => false,
}
}
#[inline]
fn ne(&self, other: &[B]) -> bool {
self[..] != other[..]
let b: Result<&[B; N], _> = other.try_into();
match b {
Ok(b) => *self != *b,
Err(_) => true,
}
}
}

Expand All @@ -38,11 +47,19 @@ where
{
#[inline]
fn eq(&self, other: &[A; N]) -> bool {
self[..] == other[..]
let b: Result<&[B; N], _> = self.try_into();
match b {
Ok(b) => *b == *other,
Err(_) => false,
}
}
#[inline]
fn ne(&self, other: &[A; N]) -> bool {
self[..] != other[..]
let b: Result<&[B; N], _> = self.try_into();
match b {
Ok(b) => *b != *other,
Err(_) => true,
}
}
}

Expand All @@ -53,11 +70,11 @@ where
{
#[inline]
fn eq(&self, other: &&[B]) -> bool {
self[..] == other[..]
*self == **other
}
#[inline]
fn ne(&self, other: &&[B]) -> bool {
self[..] != other[..]
*self != **other
}
}

Expand All @@ -68,11 +85,11 @@ where
{
#[inline]
fn eq(&self, other: &[A; N]) -> bool {
self[..] == other[..]
**self == *other
}
#[inline]
fn ne(&self, other: &[A; N]) -> bool {
self[..] != other[..]
**self != *other
}
}

Expand All @@ -83,11 +100,11 @@ where
{
#[inline]
fn eq(&self, other: &&mut [B]) -> bool {
self[..] == other[..]
*self == **other
}
#[inline]
fn ne(&self, other: &&mut [B]) -> bool {
self[..] != other[..]
*self != **other
}
}

Expand All @@ -98,11 +115,11 @@ where
{
#[inline]
fn eq(&self, other: &[A; N]) -> bool {
self[..] == other[..]
**self == *other
}
#[inline]
fn ne(&self, other: &[A; N]) -> bool {
self[..] != other[..]
**self != *other
}
}

Expand Down
44 changes: 44 additions & 0 deletions library/core/tests/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -624,3 +624,47 @@ fn array_intoiter_advance_back_by() {
assert_eq!(it.len(), 0);
assert_eq!(counter.get(), 100);
}

#[test]
fn array_mixed_equality_integers() {
let array3: [i32; 3] = [1, 2, 3];
let array3b: [i32; 3] = [3, 2, 1];
let array4: [i32; 4] = [1, 2, 3, 4];

let slice3: &[i32] = &{ array3 };
let slice3b: &[i32] = &{ array3b };
let slice4: &[i32] = &{ array4 };
assert!(array3 == slice3);
assert!(array3 != slice3b);
assert!(array3 != slice4);
assert!(slice3 == array3);
assert!(slice3b != array3);
assert!(slice4 != array3);

let mut3: &mut [i32] = &mut { array3 };
let mut3b: &mut [i32] = &mut { array3b };
let mut4: &mut [i32] = &mut { array4 };
assert!(array3 == mut3);
assert!(array3 != mut3b);
assert!(array3 != mut4);
assert!(mut3 == array3);
assert!(mut3b != array3);
assert!(mut4 != array3);
}

#[test]
fn array_mixed_equality_nans() {
let array3: [f32; 3] = [1.0, std::f32::NAN, 3.0];

let slice3: &[f32] = &{ array3 };
assert!(!(array3 == slice3));
assert!(array3 != slice3);
assert!(!(slice3 == array3));
assert!(slice3 != array3);

let mut3: &mut [f32] = &mut { array3 };
assert!(!(array3 == mut3));
assert!(array3 != mut3);
assert!(!(mut3 == array3));
assert!(mut3 != array3);
}
19 changes: 16 additions & 3 deletions src/test/codegen/slice-ref-equality.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,31 @@

// #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 instead. `is_zero_slice` was
// but we now generate it as a load+icmp instead. `is_zero_slice` was
// tweaked to still test the case of comparison against a slice,
// and `is_zero_array` tests the new array-specific behaviour.
// The optimization was then extended to short slice-to-array comparisons,
// so the first test here now has a long slice to still get the bcmp.

// CHECK-LABEL: @is_zero_slice
// CHECK-LABEL: @is_zero_slice_long
#[no_mangle]
pub fn is_zero_slice(data: &[u8; 4]) -> bool {
pub fn is_zero_slice_long(data: &[u8; 456]) -> bool {
// CHECK: :
// CHECK-NEXT: %{{.+}} = getelementptr {{.+}}
// CHECK-NEXT: %[[BCMP:.+]] = tail call i32 @{{bcmp|memcmp}}({{.+}})
// CHECK-NEXT: %[[EQ:.+]] = icmp eq i32 %[[BCMP]], 0
// CHECK-NEXT: ret i1 %[[EQ]]
&data[..] == [0; 456]
}

// CHECK-LABEL: @is_zero_slice_short
#[no_mangle]
pub fn is_zero_slice_short(data: &[u8; 4]) -> bool {
// CHECK: :
// CHECK-NEXT: %[[PTR:.+]] = bitcast [4 x i8]* {{.+}} to i32*
// CHECK-NEXT: %[[LOAD:.+]] = load i32, i32* %[[PTR]], align 1
// CHECK-NEXT: %[[EQ:.+]] = icmp eq i32 %[[LOAD]], 0
// CHECK-NEXT: ret i1 %[[EQ]]
&data[..] == [0; 4]
}

Expand Down

0 comments on commit 7abab1e

Please sign in to comment.