From 2456495a260827217d3c612d6c577c2f165c61eb Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 30 May 2021 10:25:41 -0700 Subject: Stop generating `alloca`s+`memcmp` for simple array equality --- src/test/codegen/array-equality.rs | 36 ++++++++++++++++++++++ src/test/codegen/slice-ref-equality.rs | 19 ++++++++++-- .../intrinsics/intrinsic-raw_eq-const-padding.rs | 12 ++++++++ .../intrinsic-raw_eq-const-padding.stderr | 21 +++++++++++++ src/test/ui/intrinsics/intrinsic-raw_eq-const.rs | 27 ++++++++++++++++ 5 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 src/test/codegen/array-equality.rs create mode 100644 src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.rs create mode 100644 src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.stderr create mode 100644 src/test/ui/intrinsics/intrinsic-raw_eq-const.rs (limited to 'src') diff --git a/src/test/codegen/array-equality.rs b/src/test/codegen/array-equality.rs new file mode 100644 index 00000000000..6a9fb5c8f86 --- /dev/null +++ b/src/test/codegen/array-equality.rs @@ -0,0 +1,36 @@ +// compile-flags: -O +// only-x86_64 + +#![crate_type = "lib"] + +// CHECK-LABEL: @array_eq_value +#[no_mangle] +pub fn array_eq_value(a: [u16; 6], b: [u16; 6]) -> bool { + // CHECK-NEXT: start: + // CHECK-NEXT: %2 = icmp eq i96 %0, %1 + // CHECK-NEXT: ret i1 %2 + a == b +} + +// CHECK-LABEL: @array_eq_ref +#[no_mangle] +pub fn array_eq_ref(a: &[u16; 6], b: &[u16; 6]) -> bool { + // CHECK: start: + // CHECK: load i96, i96* %{{.+}}, align 2 + // CHECK: load i96, i96* %{{.+}}, align 2 + // CHECK: icmp eq i96 + // CHECK-NEXT: ret + a == b +} + +// CHECK-LABEL: @array_eq_long +#[no_mangle] +pub fn array_eq_long(a: &[u16; 1234], b: &[u16; 1234]) -> bool { + // CHECK-NEXT: start: + // CHECK-NEXT: bitcast + // CHECK-NEXT: bitcast + // CHECK-NEXT: %[[CMP:.+]] = tail call i32 @{{bcmp|memcmp}}(i8* nonnull dereferenceable(2468) %{{.+}}, i8* nonnull dereferenceable(2468) %{{.+}}, i64 2468) + // CHECK-NEXT: %[[EQ:.+]] = icmp eq i32 %[[CMP]], 0 + // CHECK-NEXT: ret i1 %[[EQ]] + a == b +} diff --git a/src/test/codegen/slice-ref-equality.rs b/src/test/codegen/slice-ref-equality.rs index acc7879e7b1..1f99ac7342b 100644 --- a/src/test/codegen/slice-ref-equality.rs +++ b/src/test/codegen/slice-ref-equality.rs @@ -2,15 +2,30 @@ #![crate_type = "lib"] -// #71602: check that slice equality just generates a single bcmp +// #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 +// tweaked to still test the case of comparison against a slice, +// and `is_zero_array` tests the new array-specific behaviour. // CHECK-LABEL: @is_zero_slice #[no_mangle] pub fn is_zero_slice(data: &[u8; 4]) -> bool { - // CHECK: start: + // 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; 4] +} + +// CHECK-LABEL: @is_zero_array +#[no_mangle] +pub fn is_zero_array(data: &[u8; 4]) -> bool { + // CHECK: start: + // 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] } diff --git a/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.rs b/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.rs new file mode 100644 index 00000000000..ec1c47cfaea --- /dev/null +++ b/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.rs @@ -0,0 +1,12 @@ +#![feature(core_intrinsics)] +#![feature(const_intrinsic_raw_eq)] +#![deny(const_err)] + +const BAD_RAW_EQ_CALL: bool = unsafe { + std::intrinsics::raw_eq(&(1_u8, 2_u16), &(1_u8, 2_u16)) +//~^ ERROR any use of this value will cause an error +//~| WARNING this was previously accepted by the compiler but is being phased out +}; + +pub fn main() { +} diff --git a/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.stderr b/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.stderr new file mode 100644 index 00000000000..74df99a69d1 --- /dev/null +++ b/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.stderr @@ -0,0 +1,21 @@ +error: any use of this value will cause an error + --> $DIR/intrinsic-raw_eq-const-padding.rs:6:5 + | +LL | / const BAD_RAW_EQ_CALL: bool = unsafe { +LL | | std::intrinsics::raw_eq(&(1_u8, 2_u16), &(1_u8, 2_u16)) + | | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reading 4 bytes of memory starting at alloc2, but 1 byte is uninitialized starting at alloc2+0x1, and this operation requires initialized memory +LL | | +LL | | +LL | | }; + | |__- + | +note: the lint level is defined here + --> $DIR/intrinsic-raw_eq-const-padding.rs:3:9 + | +LL | #![deny(const_err)] + | ^^^^^^^^^ + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #71800 + +error: aborting due to previous error + diff --git a/src/test/ui/intrinsics/intrinsic-raw_eq-const.rs b/src/test/ui/intrinsics/intrinsic-raw_eq-const.rs new file mode 100644 index 00000000000..8ea95467302 --- /dev/null +++ b/src/test/ui/intrinsics/intrinsic-raw_eq-const.rs @@ -0,0 +1,27 @@ +// run-pass + +#![feature(core_intrinsics)] +#![feature(const_intrinsic_raw_eq)] +#![deny(const_err)] + +pub fn main() { + use std::intrinsics::raw_eq; + + const RAW_EQ_I32_TRUE: bool = unsafe { raw_eq(&42_i32, &42) }; + assert!(RAW_EQ_I32_TRUE); + + const RAW_EQ_I32_FALSE: bool = unsafe { raw_eq(&4_i32, &2) }; + assert!(!RAW_EQ_I32_FALSE); + + const RAW_EQ_CHAR_TRUE: bool = unsafe { raw_eq(&'a', &'a') }; + assert!(RAW_EQ_CHAR_TRUE); + + const RAW_EQ_CHAR_FALSE: bool = unsafe { raw_eq(&'a', &'A') }; + assert!(!RAW_EQ_CHAR_FALSE); + + const RAW_EQ_ARRAY_TRUE: bool = unsafe { raw_eq(&[13_u8, 42], &[13, 42]) }; + assert!(RAW_EQ_ARRAY_TRUE); + + const RAW_EQ_ARRAY_FALSE: bool = unsafe { raw_eq(&[13_u8, 42], &[42, 13]) }; + assert!(!RAW_EQ_ARRAY_FALSE); +} -- cgit 1.4.1-3-g733a5 From 039a3bafecb42b51c2cc5f1bc1e0b0109873b729 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 30 May 2021 21:27:29 -0700 Subject: Add another codegen test, array_eq_zero Showing that this avoids an alloca and private constant. --- src/test/codegen/array-equality.rs | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'src') diff --git a/src/test/codegen/array-equality.rs b/src/test/codegen/array-equality.rs index 6a9fb5c8f86..aa56e32e26c 100644 --- a/src/test/codegen/array-equality.rs +++ b/src/test/codegen/array-equality.rs @@ -34,3 +34,12 @@ pub fn array_eq_long(a: &[u16; 1234], b: &[u16; 1234]) -> bool { // CHECK-NEXT: ret i1 %[[EQ]] a == b } + +// CHECK-LABEL: @array_eq_zero(i128 %0) +#[no_mangle] +pub fn array_eq_zero(x: [u16; 8]) -> bool { + // CHECK-NEXT: start: + // CHECK-NEXT: %[[EQ:.+]] = icmp eq i128 %0, 0 + // CHECK-NEXT: ret i1 %[[EQ]] + x == [0; 8] +} -- cgit 1.4.1-3-g733a5 From 07fb5ee78f4f251637c5c4414982a8c6e32e186d Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Wed, 2 Jun 2021 23:35:30 -0700 Subject: Adjust the threshold to look at the ABI, not just the size --- compiler/rustc_codegen_llvm/src/intrinsic.rs | 36 ++++++++++++++++++---------- src/test/codegen/array-equality.rs | 12 ++++++++++ 2 files changed, 36 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index 615295e96e1..9a968659e2f 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -297,28 +297,40 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { } sym::raw_eq => { + use abi::Abi::*; let tp_ty = substs.type_at(0); - let (size, align) = self.size_and_align_of(tp_ty); + let layout = self.layout_of(tp_ty).layout; + let use_integer_compare = match layout.abi { + Scalar(_) | ScalarPair(_, _) => true, + Uninhabited | Vector { .. } => false, + Aggregate { .. } => { + // For rusty ABIs, small aggregates are actually passed + // as `RegKind::Integer` (see `FnAbi::adjust_for_abi`), + // so we re-use that same threshold here. + layout.size <= self.data_layout().pointer_size * 2 + } + }; + let a = args[0].immediate(); let b = args[1].immediate(); - if size.bytes() == 0 { + if layout.size.bytes() == 0 { self.const_bool(true) - } else if size > self.data_layout().pointer_size * 4 { + } else if use_integer_compare { + let integer_ty = self.type_ix(layout.size.bits()); + let ptr_ty = self.type_ptr_to(integer_ty); + let a_ptr = self.bitcast(a, ptr_ty); + let a_val = self.load(a_ptr, layout.align.abi); + let b_ptr = self.bitcast(b, ptr_ty); + let b_val = self.load(b_ptr, layout.align.abi); + self.icmp(IntPredicate::IntEQ, a_val, b_val) + } else { let i8p_ty = self.type_i8p(); let a_ptr = self.bitcast(a, i8p_ty); let b_ptr = self.bitcast(b, i8p_ty); - let n = self.const_usize(size.bytes()); + let n = self.const_usize(layout.size.bytes()); let llfn = self.get_intrinsic("memcmp"); let cmp = self.call(llfn, &[a_ptr, b_ptr, n], None); self.icmp(IntPredicate::IntEQ, cmp, self.const_i32(0)) - } else { - let integer_ty = self.type_ix(size.bits()); - let ptr_ty = self.type_ptr_to(integer_ty); - let a_ptr = self.bitcast(a, ptr_ty); - let a_val = self.load(a_ptr, align); - let b_ptr = self.bitcast(b, ptr_ty); - let b_val = self.load(b_ptr, align); - self.icmp(IntPredicate::IntEQ, a_val, b_val) } } diff --git a/src/test/codegen/array-equality.rs b/src/test/codegen/array-equality.rs index aa56e32e26c..4b60fa4b0bf 100644 --- a/src/test/codegen/array-equality.rs +++ b/src/test/codegen/array-equality.rs @@ -23,6 +23,18 @@ pub fn array_eq_ref(a: &[u16; 6], b: &[u16; 6]) -> bool { a == b } +// CHECK-LABEL: @array_eq_value_still_passed_by_pointer +#[no_mangle] +pub fn array_eq_value_still_passed_by_pointer(a: [u16; 9], b: [u16; 9]) -> bool { + // CHECK-NEXT: start: + // CHECK-NEXT: bitcast + // CHECK-NEXT: bitcast + // CHECK-NEXT: %[[CMP:.+]] = tail call i32 @{{bcmp|memcmp}}(i8* nonnull dereferenceable(18) %{{.+}}, i8* nonnull dereferenceable(18) %{{.+}}, i64 18) + // CHECK-NEXT: %[[EQ:.+]] = icmp eq i32 %[[CMP]], 0 + // CHECK-NEXT: ret i1 %[[EQ]] + a == b +} + // CHECK-LABEL: @array_eq_long #[no_mangle] pub fn array_eq_long(a: &[u16; 1234], b: &[u16; 1234]) -> bool { -- cgit 1.4.1-3-g733a5 From d0644947a3b093bfc09452ecd14291cea5dd8f14 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Thu, 8 Jul 2021 15:16:37 -0700 Subject: Bless a UI test --- .../ui/intrinsics/intrinsic-raw_eq-const-padding.rs | 3 +-- .../intrinsics/intrinsic-raw_eq-const-padding.stderr | 20 ++++---------------- 2 files changed, 5 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.rs b/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.rs index ec1c47cfaea..a205a8730a0 100644 --- a/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.rs +++ b/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.rs @@ -4,8 +4,7 @@ const BAD_RAW_EQ_CALL: bool = unsafe { std::intrinsics::raw_eq(&(1_u8, 2_u16), &(1_u8, 2_u16)) -//~^ ERROR any use of this value will cause an error -//~| WARNING this was previously accepted by the compiler but is being phased out +//~^ ERROR evaluation of constant value failed }; pub fn main() { diff --git a/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.stderr b/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.stderr index 74df99a69d1..3a1b0900127 100644 --- a/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.stderr +++ b/src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.stderr @@ -1,21 +1,9 @@ -error: any use of this value will cause an error +error[E0080]: evaluation of constant value failed --> $DIR/intrinsic-raw_eq-const-padding.rs:6:5 | -LL | / const BAD_RAW_EQ_CALL: bool = unsafe { -LL | | std::intrinsics::raw_eq(&(1_u8, 2_u16), &(1_u8, 2_u16)) - | | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reading 4 bytes of memory starting at alloc2, but 1 byte is uninitialized starting at alloc2+0x1, and this operation requires initialized memory -LL | | -LL | | -LL | | }; - | |__- - | -note: the lint level is defined here - --> $DIR/intrinsic-raw_eq-const-padding.rs:3:9 - | -LL | #![deny(const_err)] - | ^^^^^^^^^ - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #71800 +LL | std::intrinsics::raw_eq(&(1_u8, 2_u16), &(1_u8, 2_u16)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reading 4 bytes of memory starting at alloc2, but 1 byte is uninitialized starting at alloc2+0x1, and this operation requires initialized memory error: aborting due to previous error +For more information about this error, try `rustc --explain E0080`. -- cgit 1.4.1-3-g733a5