From 3b16c0467677dcd3b7f46c028446343dd699c4ba Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 23 Oct 2022 17:02:50 -0700 Subject: [PATCH 1/2] `unchecked_{shl|shr}` should use `u32` as the RHS --- library/core/src/lib.rs | 1 + library/core/src/num/int_macros.rs | 16 ++++--- library/core/src/num/mod.rs | 1 + library/core/src/num/uint_macros.rs | 16 ++++--- src/test/codegen/unchecked_shifts.rs | 65 ++++++++++++++++++++++++++++ 5 files changed, 87 insertions(+), 12 deletions(-) create mode 100644 src/test/codegen/unchecked_shifts.rs diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 9cbfbbb9f399c..357a2ea6775cf 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -130,6 +130,7 @@ #![feature(const_pin)] #![feature(const_ptr_sub_ptr)] #![feature(const_replace)] +#![feature(const_result_drop)] #![feature(const_ptr_as_ref)] #![feature(const_ptr_is_null)] #![feature(const_ptr_read)] diff --git a/library/core/src/num/int_macros.rs b/library/core/src/num/int_macros.rs index 81f050cb283d4..451484df99f23 100644 --- a/library/core/src/num/int_macros.rs +++ b/library/core/src/num/int_macros.rs @@ -757,10 +757,11 @@ macro_rules! int_impl { #[rustc_const_unstable(feature = "const_inherent_unchecked_arith", issue = "85122")] #[inline(always)] #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces - pub const unsafe fn unchecked_shl(self, rhs: Self) -> Self { + pub const unsafe fn unchecked_shl(self, rhs: u32) -> Self { // SAFETY: the caller must uphold the safety contract for // `unchecked_shl`. - unsafe { intrinsics::unchecked_shl(self, rhs) } + // Any legal shift amount is losslessly representable in the self type. + unsafe { intrinsics::unchecked_shl(self, rhs.try_into().ok().unwrap_unchecked()) } } /// Checked shift right. Computes `self >> rhs`, returning `None` if `rhs` is @@ -804,10 +805,11 @@ macro_rules! int_impl { #[rustc_const_unstable(feature = "const_inherent_unchecked_arith", issue = "85122")] #[inline(always)] #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces - pub const unsafe fn unchecked_shr(self, rhs: Self) -> Self { + pub const unsafe fn unchecked_shr(self, rhs: u32) -> Self { // SAFETY: the caller must uphold the safety contract for // `unchecked_shr`. - unsafe { intrinsics::unchecked_shr(self, rhs) } + // Any legal shift amount is losslessly representable in the self type. + unsafe { intrinsics::unchecked_shr(self, rhs.try_into().ok().unwrap_unchecked()) } } /// Checked absolute value. Computes `self.abs()`, returning `None` if @@ -1354,11 +1356,12 @@ macro_rules! int_impl { #[must_use = "this returns the result of the operation, \ without modifying the original"] #[inline(always)] + #[rustc_allow_const_fn_unstable(const_inherent_unchecked_arith)] pub const fn wrapping_shl(self, rhs: u32) -> Self { // SAFETY: the masking by the bitsize of the type ensures that we do not shift // out of bounds unsafe { - intrinsics::unchecked_shl(self, (rhs & ($BITS - 1)) as $SelfT) + self.unchecked_shl(rhs & ($BITS - 1)) } } @@ -1383,11 +1386,12 @@ macro_rules! int_impl { #[must_use = "this returns the result of the operation, \ without modifying the original"] #[inline(always)] + #[rustc_allow_const_fn_unstable(const_inherent_unchecked_arith)] pub const fn wrapping_shr(self, rhs: u32) -> Self { // SAFETY: the masking by the bitsize of the type ensures that we do not shift // out of bounds unsafe { - intrinsics::unchecked_shr(self, (rhs & ($BITS - 1)) as $SelfT) + self.unchecked_shr(rhs & ($BITS - 1)) } } diff --git a/library/core/src/num/mod.rs b/library/core/src/num/mod.rs index 311c5fa5b6834..b2328b001de90 100644 --- a/library/core/src/num/mod.rs +++ b/library/core/src/num/mod.rs @@ -3,6 +3,7 @@ #![stable(feature = "rust1", since = "1.0.0")] use crate::ascii; +use crate::convert::TryInto; use crate::error::Error; use crate::intrinsics; use crate::mem; diff --git a/library/core/src/num/uint_macros.rs b/library/core/src/num/uint_macros.rs index 93f65c5c7aaf3..1b728af79d99b 100644 --- a/library/core/src/num/uint_macros.rs +++ b/library/core/src/num/uint_macros.rs @@ -901,10 +901,11 @@ macro_rules! uint_impl { #[rustc_const_unstable(feature = "const_inherent_unchecked_arith", issue = "85122")] #[inline(always)] #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces - pub const unsafe fn unchecked_shl(self, rhs: Self) -> Self { + pub const unsafe fn unchecked_shl(self, rhs: u32) -> Self { // SAFETY: the caller must uphold the safety contract for // `unchecked_shl`. - unsafe { intrinsics::unchecked_shl(self, rhs) } + // Any legal shift amount is losslessly representable in the self type. + unsafe { intrinsics::unchecked_shl(self, rhs.try_into().ok().unwrap_unchecked()) } } /// Checked shift right. Computes `self >> rhs`, returning `None` @@ -948,10 +949,11 @@ macro_rules! uint_impl { #[rustc_const_unstable(feature = "const_inherent_unchecked_arith", issue = "85122")] #[inline(always)] #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces - pub const unsafe fn unchecked_shr(self, rhs: Self) -> Self { + pub const unsafe fn unchecked_shr(self, rhs: u32) -> Self { // SAFETY: the caller must uphold the safety contract for // `unchecked_shr`. - unsafe { intrinsics::unchecked_shr(self, rhs) } + // Any legal shift amount is losslessly representable in the self type. + unsafe { intrinsics::unchecked_shr(self, rhs.try_into().ok().unwrap_unchecked()) } } /// Checked exponentiation. Computes `self.pow(exp)`, returning `None` if @@ -1367,11 +1369,12 @@ macro_rules! uint_impl { #[must_use = "this returns the result of the operation, \ without modifying the original"] #[inline(always)] + #[rustc_allow_const_fn_unstable(const_inherent_unchecked_arith)] pub const fn wrapping_shl(self, rhs: u32) -> Self { // SAFETY: the masking by the bitsize of the type ensures that we do not shift // out of bounds unsafe { - intrinsics::unchecked_shl(self, (rhs & ($BITS - 1)) as $SelfT) + self.unchecked_shl(rhs & ($BITS - 1)) } } @@ -1399,11 +1402,12 @@ macro_rules! uint_impl { #[must_use = "this returns the result of the operation, \ without modifying the original"] #[inline(always)] + #[rustc_allow_const_fn_unstable(const_inherent_unchecked_arith)] pub const fn wrapping_shr(self, rhs: u32) -> Self { // SAFETY: the masking by the bitsize of the type ensures that we do not shift // out of bounds unsafe { - intrinsics::unchecked_shr(self, (rhs & ($BITS - 1)) as $SelfT) + self.unchecked_shr(rhs & ($BITS - 1)) } } diff --git a/src/test/codegen/unchecked_shifts.rs b/src/test/codegen/unchecked_shifts.rs new file mode 100644 index 0000000000000..d67d3a88000d8 --- /dev/null +++ b/src/test/codegen/unchecked_shifts.rs @@ -0,0 +1,65 @@ +// compile-flags: -O +// min-llvm-version: 15.0 (LLVM 13 in CI does this differently from submodule LLVM) + +#![crate_type = "lib"] +#![feature(unchecked_math)] + +// CHECK-LABEL: @unchecked_shl_unsigned_same +#[no_mangle] +pub unsafe fn unchecked_shl_unsigned_same(a: u32, b: u32) -> u32 { + // CHECK-NOT: and i32 + // CHECK: shl i32 %a, %b + // CHECK-NOT: and i32 + a.unchecked_shl(b) +} + +// CHECK-LABEL: @unchecked_shl_unsigned_smaller +#[no_mangle] +pub unsafe fn unchecked_shl_unsigned_smaller(a: u16, b: u32) -> u16 { + // This uses -DAG to avoid failing on irrelevant reorderings, + // like emitting the truncation earlier. + + // CHECK-DAG: %[[INRANGE:.+]] = icmp ult i32 %b, 65536 + // CHECK-DAG: tail call void @llvm.assume(i1 %[[INRANGE]]) + // CHECK-DAG: %[[TRUNC:.+]] = trunc i32 %b to i16 + // CHECK-DAG: shl i16 %a, %[[TRUNC]] + a.unchecked_shl(b) +} + +// CHECK-LABEL: @unchecked_shl_unsigned_bigger +#[no_mangle] +pub unsafe fn unchecked_shl_unsigned_bigger(a: u64, b: u32) -> u64 { + // CHECK: %[[EXT:.+]] = zext i32 %b to i64 + // CHECK: shl i64 %a, %[[EXT]] + a.unchecked_shl(b) +} + +// CHECK-LABEL: @unchecked_shr_signed_same +#[no_mangle] +pub unsafe fn unchecked_shr_signed_same(a: i32, b: u32) -> i32 { + // CHECK-NOT: and i32 + // CHECK: ashr i32 %a, %b + // CHECK-NOT: and i32 + a.unchecked_shr(b) +} + +// CHECK-LABEL: @unchecked_shr_signed_smaller +#[no_mangle] +pub unsafe fn unchecked_shr_signed_smaller(a: i16, b: u32) -> i16 { + // This uses -DAG to avoid failing on irrelevant reorderings, + // like emitting the truncation earlier. + + // CHECK-DAG: %[[INRANGE:.+]] = icmp ult i32 %b, 32768 + // CHECK-DAG: tail call void @llvm.assume(i1 %[[INRANGE]]) + // CHECK-DAG: %[[TRUNC:.+]] = trunc i32 %b to i16 + // CHECK-DAG: ashr i16 %a, %[[TRUNC]] + a.unchecked_shr(b) +} + +// CHECK-LABEL: @unchecked_shr_signed_bigger +#[no_mangle] +pub unsafe fn unchecked_shr_signed_bigger(a: i64, b: u32) -> i64 { + // CHECK: %[[EXT:.+]] = zext i32 %b to i64 + // CHECK: ashr i64 %a, %[[EXT]] + a.unchecked_shr(b) +} From 9d4b1f98e6761e9a6c77a840fcaec1aea0741669 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Wed, 16 Nov 2022 15:58:43 -0800 Subject: [PATCH 2/2] Ignore the unchecked-shifts codegen test in debug-assertions builds --- src/test/codegen/unchecked_shifts.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/codegen/unchecked_shifts.rs b/src/test/codegen/unchecked_shifts.rs index d67d3a88000d8..60d0cb09acaf9 100644 --- a/src/test/codegen/unchecked_shifts.rs +++ b/src/test/codegen/unchecked_shifts.rs @@ -1,5 +1,6 @@ // compile-flags: -O // min-llvm-version: 15.0 (LLVM 13 in CI does this differently from submodule LLVM) +// ignore-debug (because unchecked is checked in debug) #![crate_type = "lib"] #![feature(unchecked_math)]