From c1234e2cb31786f3a50ab51e6c8c0ae0a9c83af7 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Thu, 16 Jan 2025 20:13:48 -0800 Subject: [PATCH 1/2] Emit fewer `assume`s now that we have `range` metadata on parameters We still need the `assume` for the *target* type's range, but we no longer need it for the *source* type's range. Admittedly there's one test not properly handled by LLVM today, but it's synthetic, so I'd still be fine doing this and just updating the test once LLVM fixes the bug. All the other optimization tests still pass. Hopefully this means less crud for LLVM to churn through in `opt` builds... --- compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 34 +++--- tests/codegen/intrinsics/transmute-niched.rs | 111 +++++++++---------- tests/codegen/issues/issue-119422.rs | 1 + tests/codegen/transmute-optimized.rs | 20 +++- tests/codegen/vec-in-place.rs | 30 ----- 5 files changed, 91 insertions(+), 105 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index 31793641d75ed..7ec6f1d09e527 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -257,13 +257,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { if let OperandValueKind::Immediate(to_scalar) = cast_kind && from_scalar.size(self.cx) == to_scalar.size(self.cx) { - let from_backend_ty = bx.backend_type(operand.layout); let to_backend_ty = bx.backend_type(cast); Some(OperandValue::Immediate(self.transmute_immediate( bx, imm, from_scalar, - from_backend_ty, to_scalar, to_backend_ty, ))) @@ -279,13 +277,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { && in_a.size(self.cx) == out_a.size(self.cx) && in_b.size(self.cx) == out_b.size(self.cx) { - let in_a_ibty = bx.scalar_pair_element_backend_type(operand.layout, 0, false); - let in_b_ibty = bx.scalar_pair_element_backend_type(operand.layout, 1, false); let out_a_ibty = bx.scalar_pair_element_backend_type(cast, 0, false); let out_b_ibty = bx.scalar_pair_element_backend_type(cast, 1, false); Some(OperandValue::Pair( - self.transmute_immediate(bx, imm_a, in_a, in_a_ibty, out_a, out_a_ibty), - self.transmute_immediate(bx, imm_b, in_b, in_b_ibty, out_b, out_b_ibty), + self.transmute_immediate(bx, imm_a, in_a, out_a, out_a_ibty), + self.transmute_immediate(bx, imm_b, in_b, out_b, out_b_ibty), )) } else { None @@ -309,12 +305,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { ) -> Option { use abi::Primitive::*; - // When scalars are passed by value, there's no metadata recording their - // valid ranges. For example, `char`s are passed as just `i32`, with no - // way for LLVM to know that they're 0x10FFFF at most. Thus we assume - // the range of the input value too, not just the output range. - self.assume_scalar_range(bx, imm, from_scalar, from_backend_ty); - imm = match (from_scalar.primitive(), to_scalar.primitive()) { (Int(_, is_signed), Int(..)) => bx.intcast(imm, to_backend_ty, is_signed), (Float(_), Float(_)) => { @@ -356,7 +346,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { bx: &mut Bx, mut imm: Bx::Value, from_scalar: abi::Scalar, - from_backend_ty: Bx::Type, to_scalar: abi::Scalar, to_backend_ty: Bx::Type, ) -> Bx::Value { @@ -365,11 +354,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { use abi::Primitive::*; imm = bx.from_immediate(imm); - // When scalars are passed by value, there's no metadata recording their - // valid ranges. For example, `char`s are passed as just `i32`, with no - // way for LLVM to know that they're 0x10FFFF at most. Thus we assume - // the range of the input value too, not just the output range. - self.assume_scalar_range(bx, imm, from_scalar, from_backend_ty); + // We used to `assume` the `from_scalar` here too, but that's no longer needed + // because if we have a scalar, we must already know its range. Either + // 1) It's a parameter with `range` parameter metadata, + // 2) It's something we `load`ed with `!range` metadata, or + // 3) It's something we transmuted and already `assume`d the range. + // And thus in all those cases another `assume` is just wasteful. + // (Case 1 didn't used to be covered, and thus the `assume` was needed.) imm = match (from_scalar.primitive(), to_scalar.primitive()) { (Int(..) | Float(_), Int(..) | Float(_)) => bx.bitcast(imm, to_backend_ty), @@ -389,7 +380,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { bx.bitcast(int_imm, to_backend_ty) } }; + + // This `assume` remains important for cases like + // transmute::(x) == 0 + // since it's never passed to something with parameter metadata (especially + // after MIR inlining) so the only way to tell the backend about the + // constraint that the `transmute` introduced is to `assume` it. self.assume_scalar_range(bx, imm, to_scalar, to_backend_ty); + imm = bx.to_immediate_scalar(imm, to_scalar); imm } diff --git a/tests/codegen/intrinsics/transmute-niched.rs b/tests/codegen/intrinsics/transmute-niched.rs index f5b7bd2efea8e..278c8c088e10e 100644 --- a/tests/codegen/intrinsics/transmute-niched.rs +++ b/tests/codegen/intrinsics/transmute-niched.rs @@ -1,5 +1,6 @@ //@ revisions: OPT DBG //@ [OPT] compile-flags: -C opt-level=3 -C no-prepopulate-passes +//@ [OPT] min-llvm-version: 19 (for range parameter metadata) //@ [DBG] compile-flags: -C opt-level=0 -C no-prepopulate-passes //@ only-64bit (so I don't need to worry about usize) #![crate_type = "lib"] @@ -17,26 +18,25 @@ pub enum SmallEnum { // CHECK-LABEL: @check_to_enum( #[no_mangle] pub unsafe fn check_to_enum(x: i8) -> SmallEnum { + // CHECK-NOT: icmp + // CHECK-NOT: assume // OPT: %0 = icmp uge i8 %x, 10 // OPT: call void @llvm.assume(i1 %0) // OPT: %1 = icmp ule i8 %x, 12 // OPT: call void @llvm.assume(i1 %1) - // DBG-NOT: icmp - // DBG-NOT: assume + // CHECK-NOT: icmp + // CHECK-NOT: assume // CHECK: ret i8 %x transmute(x) } // CHECK-LABEL: @check_from_enum( +// OPT-SAME: range(i8 10, 13){{.+}}%x #[no_mangle] pub unsafe fn check_from_enum(x: SmallEnum) -> i8 { - // OPT: %0 = icmp uge i8 %x, 10 - // OPT: call void @llvm.assume(i1 %0) - // OPT: %1 = icmp ule i8 %x, 12 - // OPT: call void @llvm.assume(i1 %1) - // DBG-NOT: icmp - // DBG-NOT: assume + // CHECK-NOT: icmp + // CHECK-NOT: assume // CHECK: ret i8 %x transmute(x) @@ -45,26 +45,25 @@ pub unsafe fn check_from_enum(x: SmallEnum) -> i8 { // CHECK-LABEL: @check_to_ordering( #[no_mangle] pub unsafe fn check_to_ordering(x: u8) -> std::cmp::Ordering { + // CHECK-NOT: icmp + // CHECK-NOT: assume // OPT: %0 = icmp uge i8 %x, -1 // OPT: %1 = icmp ule i8 %x, 1 // OPT: %2 = or i1 %0, %1 // OPT: call void @llvm.assume(i1 %2) - // DBG-NOT: icmp - // DBG-NOT: assume + // CHECK-NOT: icmp + // CHECK-NOT: assume // CHECK: ret i8 %x transmute(x) } // CHECK-LABEL: @check_from_ordering( +// OPT-SAME: range(i8 -1, 2){{.+}}%x #[no_mangle] pub unsafe fn check_from_ordering(x: std::cmp::Ordering) -> u8 { - // OPT: %0 = icmp uge i8 %x, -1 - // OPT: %1 = icmp ule i8 %x, 1 - // OPT: %2 = or i1 %0, %1 - // OPT: call void @llvm.assume(i1 %2) - // DBG-NOT: icmp - // DBG-NOT: assume + // CHECK-NOT: icmp + // CHECK-NOT: assume // CHECK: ret i8 %x transmute(x) @@ -96,50 +95,50 @@ pub enum Minus100ToPlus100 { } // CHECK-LABEL: @check_enum_from_char( +// OPT-SAME: range(i32 0, 1114112){{.+}}%x #[no_mangle] pub unsafe fn check_enum_from_char(x: char) -> Minus100ToPlus100 { - // OPT: %0 = icmp ule i32 %x, 1114111 - // OPT: call void @llvm.assume(i1 %0) - // OPT: %1 = icmp uge i32 %x, -100 - // OPT: %2 = icmp ule i32 %x, 100 - // OPT: %3 = or i1 %1, %2 - // OPT: call void @llvm.assume(i1 %3) - // DBG-NOT: icmp - // DBG-NOT: assume + // CHECK-NOT: icmp + // CHECK-NOT: assume + // OPT: %0 = icmp uge i32 %x, -100 + // OPT: %1 = icmp ule i32 %x, 100 + // OPT: %2 = or i1 %0, %1 + // OPT: call void @llvm.assume(i1 %2) + // CHECK-NOT: icmp + // CHECK-NOT: assume // CHECK: ret i32 %x transmute(x) } // CHECK-LABEL: @check_enum_to_char( +// OPT-SAME: range(i32 -100, 101){{.+}}%x #[no_mangle] pub unsafe fn check_enum_to_char(x: Minus100ToPlus100) -> char { - // OPT: %0 = icmp uge i32 %x, -100 - // OPT: %1 = icmp ule i32 %x, 100 - // OPT: %2 = or i1 %0, %1 - // OPT: call void @llvm.assume(i1 %2) - // OPT: %3 = icmp ule i32 %x, 1114111 - // OPT: call void @llvm.assume(i1 %3) - // DBG-NOT: icmp - // DBG-NOT: assume + // CHECK-NOT: icmp + // CHECK-NOT: assume + // OPT: %0 = icmp ule i32 %x, 1114111 + // OPT: call void @llvm.assume(i1 %0) + // CHECK-NOT: icmp + // CHECK-NOT: assume // CHECK: ret i32 %x transmute(x) } // CHECK-LABEL: @check_swap_pair( +// OPT-SAME: range(i32 0, 1114112){{.+}}%x.0 +// OPT-SAME: range(i32 1, 0){{.+}}%x.1 #[no_mangle] pub unsafe fn check_swap_pair(x: (char, NonZero)) -> (NonZero, char) { - // OPT: %0 = icmp ule i32 %x.0, 1114111 + // CHECK-NOT: icmp + // CHECK-NOT: assume + // OPT: %0 = icmp uge i32 %x.0, 1 // OPT: call void @llvm.assume(i1 %0) - // OPT: %1 = icmp uge i32 %x.0, 1 + // OPT: %1 = icmp ule i32 %x.1, 1114111 // OPT: call void @llvm.assume(i1 %1) - // OPT: %2 = icmp uge i32 %x.1, 1 - // OPT: call void @llvm.assume(i1 %2) - // OPT: %3 = icmp ule i32 %x.1, 1114111 - // OPT: call void @llvm.assume(i1 %3) - // DBG-NOT: icmp - // DBG-NOT: assume + // CHECK-NOT: icmp + // CHECK-NOT: assume // CHECK: %[[P1:.+]] = insertvalue { i32, i32 } poison, i32 %x.0, 0 // CHECK: %[[P2:.+]] = insertvalue { i32, i32 } %[[P1]], i32 %x.1, 1 // CHECK: ret { i32, i32 } %[[P2]] @@ -148,16 +147,15 @@ pub unsafe fn check_swap_pair(x: (char, NonZero)) -> (NonZero, char) { } // CHECK-LABEL: @check_bool_from_ordering( +// OPT-SAME: range(i8 -1, 2){{.+}}%x #[no_mangle] pub unsafe fn check_bool_from_ordering(x: std::cmp::Ordering) -> bool { - // OPT: %0 = icmp uge i8 %x, -1 - // OPT: %1 = icmp ule i8 %x, 1 - // OPT: %2 = or i1 %0, %1 - // OPT: call void @llvm.assume(i1 %2) - // OPT: %3 = icmp ule i8 %x, 1 - // OPT: call void @llvm.assume(i1 %3) - // DBG-NOT: icmp - // DBG-NOT: assume + // CHECK-NOT: icmp + // CHECK-NOT: assume + // OPT: %0 = icmp ule i8 %x, 1 + // OPT: call void @llvm.assume(i1 %0) + // CHECK-NOT: icmp + // CHECK-NOT: assume // CHECK: %[[R:.+]] = trunc i8 %x to i1 // CHECK: ret i1 %[[R]] @@ -165,17 +163,18 @@ pub unsafe fn check_bool_from_ordering(x: std::cmp::Ordering) -> bool { } // CHECK-LABEL: @check_bool_to_ordering( +// OPT-SAME: i1 {{.+}} %x #[no_mangle] pub unsafe fn check_bool_to_ordering(x: bool) -> std::cmp::Ordering { // CHECK: %_0 = zext i1 %x to i8 - // OPT: %0 = icmp ule i8 %_0, 1 - // OPT: call void @llvm.assume(i1 %0) - // OPT: %1 = icmp uge i8 %_0, -1 - // OPT: %2 = icmp ule i8 %_0, 1 - // OPT: %3 = or i1 %1, %2 - // OPT: call void @llvm.assume(i1 %3) - // DBG-NOT: icmp - // DBG-NOT: assume + // CHECK-NOT: icmp + // CHECK-NOT: assume + // OPT: %0 = icmp uge i8 %_0, -1 + // OPT: %1 = icmp ule i8 %_0, 1 + // OPT: %2 = or i1 %0, %1 + // OPT: call void @llvm.assume(i1 %2) + // CHECK-NOT: icmp + // CHECK-NOT: assume // CHECK: ret i8 %_0 transmute(x) diff --git a/tests/codegen/issues/issue-119422.rs b/tests/codegen/issues/issue-119422.rs index 682430a79f4a9..97b15d5ee2d4a 100644 --- a/tests/codegen/issues/issue-119422.rs +++ b/tests/codegen/issues/issue-119422.rs @@ -2,6 +2,7 @@ //! for `NonZero` integer types. //! //@ compile-flags: -O --edition=2021 -Zmerge-functions=disabled +//@ min-llvm-version: 19 (for range parameter metadata) //@ only-64bit (because the LLVM type of i64 for usize shows up) #![crate_type = "lib"] diff --git a/tests/codegen/transmute-optimized.rs b/tests/codegen/transmute-optimized.rs index 11bd0523788a1..176d2f5a2db9a 100644 --- a/tests/codegen/transmute-optimized.rs +++ b/tests/codegen/transmute-optimized.rs @@ -1,4 +1,5 @@ //@ compile-flags: -O -Z merge-functions=disabled +//@ min-llvm-version: 19 (for range parameter metadata) #![crate_type = "lib"] // This tests that LLVM can optimize based on the niches in the source or @@ -90,9 +91,18 @@ pub enum OneTwoThree { } // CHECK-LABEL: i8 @ordering_transmute_onetwothree(i8 +// CHECK-SAME: range(i8 -1, 2){{.+}}%x #[no_mangle] pub unsafe fn ordering_transmute_onetwothree(x: std::cmp::Ordering) -> OneTwoThree { - // CHECK: ret i8 1 + // FIXME: this *should* just be `ret i8 1`, but that's not happening today. + // cc + + // CHECK: %[[TEMP1:.+]] = icmp ne i8 %x, 0 + // CHECK: tail call void @llvm.assume(i1 %[[TEMP1]]) + // CHECK: %[[TEMP2:.+]] = icmp ult i8 %x, 4 + // CHECK: tail call void @llvm.assume(i1 %[[TEMP2]]) + + // CHECK: ret i8 %x std::mem::transmute(x) } @@ -110,3 +120,11 @@ pub fn char_is_negative(c: char) -> bool { let x: i32 = unsafe { std::mem::transmute(c) }; x < 0 } + +// CHECK-LABEL: i1 @transmute_to_char_is_negative(i32 +#[no_mangle] +pub fn transmute_to_char_is_negative(x: i32) -> bool { + // CHECK: ret i1 false + let _c: char = unsafe { std::mem::transmute(x) }; + x < 0 +} diff --git a/tests/codegen/vec-in-place.rs b/tests/codegen/vec-in-place.rs index e835a7ef69bd8..878a899785435 100644 --- a/tests/codegen/vec-in-place.rs +++ b/tests/codegen/vec-in-place.rs @@ -41,9 +41,6 @@ pub fn vec_iterator_cast_primitive(vec: Vec) -> Vec { // CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}}) // CHECK-NOT: loop // CHECK-NOT: call - // CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}}) - // CHECK-NOT: loop - // CHECK-NOT: call vec.into_iter().map(|e| e as u8).collect() } @@ -55,9 +52,6 @@ pub fn vec_iterator_cast_wrapper(vec: Vec) -> Vec> { // CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}}) // CHECK-NOT: loop // CHECK-NOT: call - // CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}}) - // CHECK-NOT: loop - // CHECK-NOT: call vec.into_iter().map(|e| Wrapper(e)).collect() } @@ -86,9 +80,6 @@ pub fn vec_iterator_cast_unwrap(vec: Vec>) -> Vec { // CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}}) // CHECK-NOT: loop // CHECK-NOT: call - // CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}}) - // CHECK-NOT: loop - // CHECK-NOT: call vec.into_iter().map(|e| e.0).collect() } @@ -100,9 +91,6 @@ pub fn vec_iterator_cast_aggregate(vec: Vec<[u64; 4]>) -> Vec { // CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}}) // CHECK-NOT: loop // CHECK-NOT: call - // CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}}) - // CHECK-NOT: loop - // CHECK-NOT: call vec.into_iter().map(|e| unsafe { std::mem::transmute(e) }).collect() } @@ -114,9 +102,6 @@ pub fn vec_iterator_cast_deaggregate_tra(vec: Vec) -> Vec<[u64; 4]> { // CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}}) // CHECK-NOT: loop // CHECK-NOT: call - // CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}}) - // CHECK-NOT: loop - // CHECK-NOT: call // Safety: For the purpose of this test we assume that Bar layout matches [u64; 4]. // This currently is not guaranteed for repr(Rust) types, but it happens to work here and @@ -133,9 +118,6 @@ pub fn vec_iterator_cast_deaggregate_fold(vec: Vec) -> Vec<[u64; 4]> { // CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}}) // CHECK-NOT: loop // CHECK-NOT: call - // CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}}) - // CHECK-NOT: loop - // CHECK-NOT: call // Safety: For the purpose of this test we assume that Bar layout matches [u64; 4]. // This currently is not guaranteed for repr(Rust) types, but it happens to work here and @@ -156,12 +138,6 @@ pub fn vec_iterator_cast_unwrap_drop(vec: Vec>) -> Vec { // CHECK-NOT: call // CHECK-NOT: %{{.*}} = mul // CHECK-NOT: %{{.*}} = udiv - // CHECK: call - // CHECK-SAME: void @llvm.assume(i1 %{{.+}}) - // CHECK-NOT: br i1 %{{.*}}, label %{{.*}}, label %{{.*}} - // CHECK-NOT: call - // CHECK-NOT: %{{.*}} = mul - // CHECK-NOT: %{{.*}} = udiv vec.into_iter().map(|Wrapper(e)| e).collect() } @@ -178,12 +154,6 @@ pub fn vec_iterator_cast_wrap_drop(vec: Vec) -> Vec> { // CHECK-NOT: call // CHECK-NOT: %{{.*}} = mul // CHECK-NOT: %{{.*}} = udiv - // CHECK: call - // CHECK-SAME: void @llvm.assume(i1 %{{.+}}) - // CHECK-NOT: br i1 %{{.*}}, label %{{.*}}, label %{{.*}} - // CHECK-NOT: call - // CHECK-NOT: %{{.*}} = mul - // CHECK-NOT: %{{.*}} = udiv // CHECK: ret void vec.into_iter().map(Wrapper).collect() From 8ab474a9b9759be27b6bb23eba74749fb2feda5c Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Thu, 16 Jan 2025 21:51:10 -0800 Subject: [PATCH 2/2] Emit range assumes as a single `icmp` Thank you dtcxzyw in LLVM 123278 for pointing out that we were doing this in a suboptimal way. --- compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 37 ++----------------- .../rustc_codegen_ssa/src/traits/builder.rs | 22 +++++++++++ tests/codegen/intrinsics/transmute-niched.rs | 33 ++++++++--------- tests/codegen/transmute-optimized.rs | 11 ++---- 4 files changed, 44 insertions(+), 59 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index 7ec6f1d09e527..f196d0dae8de0 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -386,19 +386,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // since it's never passed to something with parameter metadata (especially // after MIR inlining) so the only way to tell the backend about the // constraint that the `transmute` introduced is to `assume` it. - self.assume_scalar_range(bx, imm, to_scalar, to_backend_ty); + self.assume_scalar_range(bx, imm, to_scalar); imm = bx.to_immediate_scalar(imm, to_scalar); imm } - fn assume_scalar_range( - &self, - bx: &mut Bx, - imm: Bx::Value, - scalar: abi::Scalar, - backend_ty: Bx::Type, - ) { + fn assume_scalar_range(&self, bx: &mut Bx, imm: Bx::Value, scalar: abi::Scalar) { if matches!(self.cx.sess().opts.optimize, OptLevel::No) // For now, the critical niches are all over `Int`eger values. // Should floating-point values or pointers ever get more complex @@ -409,31 +403,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { return; } - let abi::WrappingRange { start, end } = scalar.valid_range(self.cx); - - if start <= end { - if start > 0 { - let low = bx.const_uint_big(backend_ty, start); - let cmp = bx.icmp(IntPredicate::IntUGE, imm, low); - bx.assume(cmp); - } - - let type_max = scalar.size(self.cx).unsigned_int_max(); - if end < type_max { - let high = bx.const_uint_big(backend_ty, end); - let cmp = bx.icmp(IntPredicate::IntULE, imm, high); - bx.assume(cmp); - } - } else { - let low = bx.const_uint_big(backend_ty, start); - let cmp_low = bx.icmp(IntPredicate::IntUGE, imm, low); - - let high = bx.const_uint_big(backend_ty, end); - let cmp_high = bx.icmp(IntPredicate::IntULE, imm, high); - - let or = bx.or(cmp_low, cmp_high); - bx.assume(or); - } + let range = scalar.valid_range(self.cx); + bx.assume_integer_range(imm, range); } pub(crate) fn codegen_rvalue_unsized( diff --git a/compiler/rustc_codegen_ssa/src/traits/builder.rs b/compiler/rustc_codegen_ssa/src/traits/builder.rs index 3ee13b19f665f..fe739ddd75d8d 100644 --- a/compiler/rustc_codegen_ssa/src/traits/builder.rs +++ b/compiler/rustc_codegen_ssa/src/traits/builder.rs @@ -217,6 +217,28 @@ pub trait BuilderMethods<'a, 'tcx>: dest: PlaceRef<'tcx, Self::Value>, ); + /// Emits an `assume` that the integer value `imm` is contained in `range`. + /// + /// This *always* emits the assumption, so you probably want to check the + /// optimization level and `Scalar::is_always_valid` before calling it. + fn assume_integer_range(&mut self, imm: Self::Value, range: WrappingRange) { + let WrappingRange { start, end } = range; + let backend_ty = self.cx().val_ty(imm); + + // Perhaps one day we'll be able to use assume operand bundles for this, + // but for now this encoding with a single icmp+assume is best per + // + let shifted = if start == 0 { + imm + } else { + let low = self.const_uint_big(backend_ty, start); + self.sub(imm, low) + }; + let width = self.const_uint_big(backend_ty, u128::wrapping_sub(end, start)); + let cmp = self.icmp(IntPredicate::IntULE, shifted, width); + self.assume(cmp); + } + fn range_metadata(&mut self, load: Self::Value, range: WrappingRange); fn nonnull_metadata(&mut self, load: Self::Value); diff --git a/tests/codegen/intrinsics/transmute-niched.rs b/tests/codegen/intrinsics/transmute-niched.rs index 278c8c088e10e..8b9b7607e6420 100644 --- a/tests/codegen/intrinsics/transmute-niched.rs +++ b/tests/codegen/intrinsics/transmute-niched.rs @@ -20,9 +20,8 @@ pub enum SmallEnum { pub unsafe fn check_to_enum(x: i8) -> SmallEnum { // CHECK-NOT: icmp // CHECK-NOT: assume - // OPT: %0 = icmp uge i8 %x, 10 - // OPT: call void @llvm.assume(i1 %0) - // OPT: %1 = icmp ule i8 %x, 12 + // OPT: %0 = sub i8 %x, 10 + // OPT: %1 = icmp ule i8 %0, 2 // OPT: call void @llvm.assume(i1 %1) // CHECK-NOT: icmp // CHECK-NOT: assume @@ -47,10 +46,9 @@ pub unsafe fn check_from_enum(x: SmallEnum) -> i8 { pub unsafe fn check_to_ordering(x: u8) -> std::cmp::Ordering { // CHECK-NOT: icmp // CHECK-NOT: assume - // OPT: %0 = icmp uge i8 %x, -1 - // OPT: %1 = icmp ule i8 %x, 1 - // OPT: %2 = or i1 %0, %1 - // OPT: call void @llvm.assume(i1 %2) + // OPT: %0 = sub i8 %x, -1 + // OPT: %1 = icmp ule i8 %0, 2 + // OPT: call void @llvm.assume(i1 %1) // CHECK-NOT: icmp // CHECK-NOT: assume // CHECK: ret i8 %x @@ -100,10 +98,9 @@ pub enum Minus100ToPlus100 { pub unsafe fn check_enum_from_char(x: char) -> Minus100ToPlus100 { // CHECK-NOT: icmp // CHECK-NOT: assume - // OPT: %0 = icmp uge i32 %x, -100 - // OPT: %1 = icmp ule i32 %x, 100 - // OPT: %2 = or i1 %0, %1 - // OPT: call void @llvm.assume(i1 %2) + // OPT: %0 = sub i32 %x, -100 + // OPT: %1 = icmp ule i32 %0, 200 + // OPT: call void @llvm.assume(i1 %1) // CHECK-NOT: icmp // CHECK-NOT: assume // CHECK: ret i32 %x @@ -133,10 +130,11 @@ pub unsafe fn check_enum_to_char(x: Minus100ToPlus100) -> char { pub unsafe fn check_swap_pair(x: (char, NonZero)) -> (NonZero, char) { // CHECK-NOT: icmp // CHECK-NOT: assume - // OPT: %0 = icmp uge i32 %x.0, 1 - // OPT: call void @llvm.assume(i1 %0) - // OPT: %1 = icmp ule i32 %x.1, 1114111 + // OPT: %0 = sub i32 %x.0, 1 + // OPT: %1 = icmp ule i32 %0, -2 // OPT: call void @llvm.assume(i1 %1) + // OPT: %2 = icmp ule i32 %x.1, 1114111 + // OPT: call void @llvm.assume(i1 %2) // CHECK-NOT: icmp // CHECK-NOT: assume // CHECK: %[[P1:.+]] = insertvalue { i32, i32 } poison, i32 %x.0, 0 @@ -169,10 +167,9 @@ pub unsafe fn check_bool_to_ordering(x: bool) -> std::cmp::Ordering { // CHECK: %_0 = zext i1 %x to i8 // CHECK-NOT: icmp // CHECK-NOT: assume - // OPT: %0 = icmp uge i8 %_0, -1 - // OPT: %1 = icmp ule i8 %_0, 1 - // OPT: %2 = or i1 %0, %1 - // OPT: call void @llvm.assume(i1 %2) + // OPT: %0 = sub i8 %_0, -1 + // OPT: %1 = icmp ule i8 %0, 2 + // OPT: call void @llvm.assume(i1 %1) // CHECK-NOT: icmp // CHECK-NOT: assume // CHECK: ret i8 %_0 diff --git a/tests/codegen/transmute-optimized.rs b/tests/codegen/transmute-optimized.rs index 176d2f5a2db9a..c7ea6de97592a 100644 --- a/tests/codegen/transmute-optimized.rs +++ b/tests/codegen/transmute-optimized.rs @@ -94,15 +94,10 @@ pub enum OneTwoThree { // CHECK-SAME: range(i8 -1, 2){{.+}}%x #[no_mangle] pub unsafe fn ordering_transmute_onetwothree(x: std::cmp::Ordering) -> OneTwoThree { - // FIXME: this *should* just be `ret i8 1`, but that's not happening today. - // cc - - // CHECK: %[[TEMP1:.+]] = icmp ne i8 %x, 0 - // CHECK: tail call void @llvm.assume(i1 %[[TEMP1]]) - // CHECK: %[[TEMP2:.+]] = icmp ult i8 %x, 4 + // CHECK: %[[TEMP1:.+]] = add nsw i8 %x, -1 + // CHECK: %[[TEMP2:.+]] = icmp ult i8 %[[TEMP1]], 3 // CHECK: tail call void @llvm.assume(i1 %[[TEMP2]]) - - // CHECK: ret i8 %x + // CHECK: ret i8 1 std::mem::transmute(x) }