From 1bcb0ec28cd169e1e80efffc824287287d374147 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Thu, 6 Apr 2023 00:36:36 -0700 Subject: [PATCH 1/2] `assume` value ranges in `transmute` Fixes #109958 --- compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 63 ++++++- tests/codegen/intrinsics/transmute-niched.rs | 184 +++++++++++++++++++ tests/codegen/intrinsics/transmute.rs | 8 +- tests/codegen/transmute-scalar.rs | 14 +- 4 files changed, 253 insertions(+), 16 deletions(-) create mode 100644 tests/codegen/intrinsics/transmute-niched.rs diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index d88226f5db053..e05d03d150fe3 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -12,6 +12,7 @@ use rustc_middle::mir::Operand; use rustc_middle::ty::cast::{CastTy, IntTy}; use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, TyAndLayout}; use rustc_middle::ty::{self, adjustment::PointerCast, Instance, Ty, TyCtxt}; +use rustc_session::config::OptLevel; use rustc_span::source_map::{Span, DUMMY_SP}; use rustc_target::abi::{self, FIRST_VARIANT}; @@ -231,10 +232,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { (ScalarOrZst::Scalar(in_scalar), ScalarOrZst::Scalar(out_scalar)) if in_scalar.size(self.cx) == out_scalar.size(self.cx) => { + let operand_bty = bx.backend_type(operand.layout); let cast_bty = bx.backend_type(cast); - Some(OperandValue::Immediate( - self.transmute_immediate(bx, imm, in_scalar, out_scalar, cast_bty), - )) + Some(OperandValue::Immediate(self.transmute_immediate( + bx, + imm, + in_scalar, + operand_bty, + out_scalar, + cast_bty, + ))) } _ => None, } @@ -250,11 +257,13 @@ 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, out_a, out_a_ibty), - self.transmute_immediate(bx, imm_b, in_b, out_b, out_b_ibty), + 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), )) } else { None @@ -273,6 +282,7 @@ 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 { @@ -280,6 +290,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { use abi::Primitive::*; imm = bx.from_immediate(imm); + self.assume_scalar_range(bx, imm, from_scalar, from_backend_ty); imm = match (from_scalar.primitive(), to_scalar.primitive()) { (Int(..) | F32 | F64, Int(..) | F32 | F64) => bx.bitcast(imm, to_backend_ty), (Pointer(..), Pointer(..)) => bx.pointercast(imm, to_backend_ty), @@ -294,10 +305,52 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { bx.bitcast(int_imm, to_backend_ty) } }; + self.assume_scalar_range(bx, imm, to_scalar, to_backend_ty); 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, + ) { + if matches!(self.cx.sess().opts.optimize, OptLevel::No | OptLevel::Less) + || !matches!(scalar.primitive(), abi::Primitive::Int(..)) + || scalar.is_always_valid(self.cx) + { + 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); + } + } + pub fn codegen_rvalue_unsized( &mut self, bx: &mut Bx, diff --git a/tests/codegen/intrinsics/transmute-niched.rs b/tests/codegen/intrinsics/transmute-niched.rs new file mode 100644 index 0000000000000..69e9b1d12062e --- /dev/null +++ b/tests/codegen/intrinsics/transmute-niched.rs @@ -0,0 +1,184 @@ +// revisions: OPT DBG +// [OPT] compile-flags: -C opt-level=3 -C no-prepopulate-passes +// [DBG] compile-flags: -C opt-level=0 -C no-prepopulate-passes +// only-64bit (so I don't need to worry about usize) +// min-llvm-version: 15.0 # this test assumes `ptr`s + +#![crate_type = "lib"] + +use std::mem::transmute; +use std::num::NonZeroU32; + +#[repr(u8)] +pub enum SmallEnum { + A = 10, + B = 11, + C = 12, +} + +// CHECK-LABEL: @check_to_enum( +#[no_mangle] +pub unsafe fn check_to_enum(x: i8) -> SmallEnum { + // 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: ret i8 %x + + transmute(x) +} + +// CHECK-LABEL: @check_from_enum( +#[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: ret i8 %x + + transmute(x) +} + +// CHECK-LABEL: @check_to_ordering( +#[no_mangle] +pub unsafe fn check_to_ordering(x: u8) -> std::cmp::Ordering { + // 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: ret i8 %x + + transmute(x) +} + +// CHECK-LABEL: @check_from_ordering( +#[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: ret i8 %x + + transmute(x) +} + +#[repr(i32)] +pub enum Minus100ToPlus100 { + A = -100, + B = -90, + C = -80, + D = -70, + E = -60, + F = -50, + G = -40, + H = -30, + I = -20, + J = -10, + K = 0, + L = 10, + M = 20, + N = 30, + O = 40, + P = 50, + Q = 60, + R = 70, + S = 80, + T = 90, + U = 100, +} + +// CHECK-LABEL: @check_enum_from_char( +#[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: ret i32 %x + + transmute(x) +} + +// CHECK-LABEL: @check_enum_to_char( +#[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: ret i32 %x + + transmute(x) +} + +// CHECK-LABEL: @check_swap_pair( +#[no_mangle] +pub unsafe fn check_swap_pair(x: (char, NonZeroU32)) -> (NonZeroU32, char) { + // OPT: %0 = icmp ule i32 %x.0, 1114111 + // OPT: call void @llvm.assume(i1 %0) + // OPT: %1 = icmp uge i32 %x.0, 1 + // 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: %[[P1:.+]] = insertvalue { i32, i32 } poison, i32 %x.0, 0 + // CHECK: %[[P2:.+]] = insertvalue { i32, i32 } %[[P1]], i32 %x.1, 1 + // CHECK: ret { i32, i32 } %[[P2]] + + transmute(x) +} + +// CHECK-LABEL: @check_bool_from_ordering( +#[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: %[[R:.+]] = trunc i8 %x to i1 + // CHECK: ret i1 %[[R]] + + transmute(x) +} + +// CHECK-LABEL: @check_bool_to_ordering( +#[no_mangle] +pub unsafe fn check_bool_to_ordering(x: bool) -> std::cmp::Ordering { + // CHECK: %0 = zext i1 %x to i8 + // OPT: %1 = icmp ule i8 %0, 1 + // OPT: call void @llvm.assume(i1 %1) + // OPT: %2 = icmp uge i8 %0, -1 + // OPT: %3 = icmp ule i8 %0, 1 + // OPT: %4 = or i1 %2, %3 + // OPT: call void @llvm.assume(i1 %4) + // DBG-NOT: icmp + // DBG-NOT: assume + // CHECK: ret i8 %0 + + transmute(x) +} diff --git a/tests/codegen/intrinsics/transmute.rs b/tests/codegen/intrinsics/transmute.rs index 57f901c671992..51c000b82ea8a 100644 --- a/tests/codegen/intrinsics/transmute.rs +++ b/tests/codegen/intrinsics/transmute.rs @@ -169,8 +169,8 @@ pub unsafe fn check_aggregate_from_bool(x: bool) -> Aggregate8 { #[no_mangle] pub unsafe fn check_byte_to_bool(x: u8) -> bool { // CHECK-NOT: alloca - // CHECK: %0 = trunc i8 %x to i1 - // CHECK: ret i1 %0 + // CHECK: %[[R:.+]] = trunc i8 %x to i1 + // CHECK: ret i1 %[[R]] transmute(x) } @@ -178,8 +178,8 @@ pub unsafe fn check_byte_to_bool(x: u8) -> bool { #[no_mangle] pub unsafe fn check_byte_from_bool(x: bool) -> u8 { // CHECK-NOT: alloca - // CHECK: %0 = zext i1 %x to i8 - // CHECK: ret i8 %0 + // CHECK: %[[R:.+]] = zext i1 %x to i8 + // CHECK: ret i8 %[[R:.+]] transmute(x) } diff --git a/tests/codegen/transmute-scalar.rs b/tests/codegen/transmute-scalar.rs index af2cef472ec6a..a0894a505c7c6 100644 --- a/tests/codegen/transmute-scalar.rs +++ b/tests/codegen/transmute-scalar.rs @@ -1,4 +1,4 @@ -// compile-flags: -O -C no-prepopulate-passes +// compile-flags: -C opt-level=0 -C no-prepopulate-passes // min-llvm-version: 15.0 # this test assumes `ptr`s and thus no `pointercast`s #![crate_type = "lib"] @@ -10,7 +10,7 @@ // However, `bitcast`s and `ptrtoint`s and `inttoptr`s are still worth doing when // that allows us to avoid the `alloca`s entirely; see `rvalue_creates_operand`. -// CHECK-LABEL: define{{.*}}i32 @f32_to_bits(float noundef %x) +// CHECK-LABEL: define{{.*}}i32 @f32_to_bits(float %x) // CHECK: %0 = bitcast float %x to i32 // CHECK-NEXT: ret i32 %0 #[no_mangle] @@ -18,7 +18,7 @@ pub fn f32_to_bits(x: f32) -> u32 { unsafe { std::mem::transmute(x) } } -// CHECK-LABEL: define{{.*}}i8 @bool_to_byte(i1 noundef zeroext %b) +// CHECK-LABEL: define{{.*}}i8 @bool_to_byte(i1 zeroext %b) // CHECK: %0 = zext i1 %b to i8 // CHECK-NEXT: ret i8 %0 #[no_mangle] @@ -26,7 +26,7 @@ pub fn bool_to_byte(b: bool) -> u8 { unsafe { std::mem::transmute(b) } } -// CHECK-LABEL: define{{.*}}noundef zeroext i1 @byte_to_bool(i8 noundef %byte) +// CHECK-LABEL: define{{.*}}zeroext i1 @byte_to_bool(i8 %byte) // CHECK: %0 = trunc i8 %byte to i1 // CHECK-NEXT: ret i1 %0 #[no_mangle] @@ -34,14 +34,14 @@ pub unsafe fn byte_to_bool(byte: u8) -> bool { std::mem::transmute(byte) } -// CHECK-LABEL: define{{.*}}ptr @ptr_to_ptr(ptr noundef %p) +// CHECK-LABEL: define{{.*}}ptr @ptr_to_ptr(ptr %p) // CHECK: ret ptr %p #[no_mangle] pub fn ptr_to_ptr(p: *mut u16) -> *mut u8 { unsafe { std::mem::transmute(p) } } -// CHECK: define{{.*}}[[USIZE:i[0-9]+]] @ptr_to_int(ptr noundef %p) +// CHECK: define{{.*}}[[USIZE:i[0-9]+]] @ptr_to_int(ptr %p) // CHECK: %0 = ptrtoint ptr %p to [[USIZE]] // CHECK-NEXT: ret [[USIZE]] %0 #[no_mangle] @@ -49,7 +49,7 @@ pub fn ptr_to_int(p: *mut u16) -> usize { unsafe { std::mem::transmute(p) } } -// CHECK: define{{.*}}ptr @int_to_ptr([[USIZE]] noundef %i) +// CHECK: define{{.*}}ptr @int_to_ptr([[USIZE]] %i) // CHECK: %0 = inttoptr [[USIZE]] %i to ptr // CHECK-NEXT: ret ptr %0 #[no_mangle] From baf98e7515c4736821f99a263e777fd4a7f18cbf Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Wed, 19 Apr 2023 23:14:28 -0700 Subject: [PATCH 2/2] Add transmute optimization tests and some extra comments --- compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 9 ++ tests/codegen/transmute-optimized.rs | 109 +++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 tests/codegen/transmute-optimized.rs diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index e05d03d150fe3..bd11d47500a55 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -290,7 +290,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); + imm = match (from_scalar.primitive(), to_scalar.primitive()) { (Int(..) | F32 | F64, Int(..) | F32 | F64) => bx.bitcast(imm, to_backend_ty), (Pointer(..), Pointer(..)) => bx.pointercast(imm, to_backend_ty), @@ -318,6 +324,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { backend_ty: Bx::Type, ) { if matches!(self.cx.sess().opts.optimize, OptLevel::No | OptLevel::Less) + // For now, the critical niches are all over `Int`eger values. + // Should floating-point values or pointers ever get more complex + // niches, then this code will probably want to handle them too. || !matches!(scalar.primitive(), abi::Primitive::Int(..)) || scalar.is_always_valid(self.cx) { diff --git a/tests/codegen/transmute-optimized.rs b/tests/codegen/transmute-optimized.rs new file mode 100644 index 0000000000000..461dd550cd7e8 --- /dev/null +++ b/tests/codegen/transmute-optimized.rs @@ -0,0 +1,109 @@ +// compile-flags: -O -Z merge-functions=disabled +// min-llvm-version: 15.0 # this test uses `ptr`s +// ignore-debug + +#![crate_type = "lib"] + +// This tests that LLVM can optimize based on the niches in the source or +// destination types for transmutes. + +#[repr(u32)] +pub enum AlwaysZero32 { X = 0 } + +// CHECK-LABEL: i32 @issue_109958(i32 +#[no_mangle] +pub fn issue_109958(x: AlwaysZero32) -> i32 { + // CHECK: ret i32 0 + unsafe { std::mem::transmute(x) } +} + +// CHECK-LABEL: i1 @reference_is_null(ptr +#[no_mangle] +pub fn reference_is_null(x: &i32) -> bool { + // CHECK: ret i1 false + let p: *const i32 = unsafe { std::mem::transmute(x) }; + p.is_null() +} + +// CHECK-LABEL: i1 @non_null_is_null(ptr +#[no_mangle] +pub fn non_null_is_null(x: std::ptr::NonNull) -> bool { + // CHECK: ret i1 false + let p: *const i32 = unsafe { std::mem::transmute(x) }; + p.is_null() +} + +// CHECK-LABEL: i1 @non_zero_is_null( +#[no_mangle] +pub fn non_zero_is_null(x: std::num::NonZeroUsize) -> bool { + // CHECK: ret i1 false + let p: *const i32 = unsafe { std::mem::transmute(x) }; + p.is_null() +} + +// CHECK-LABEL: i1 @non_null_is_zero(ptr +#[no_mangle] +pub fn non_null_is_zero(x: std::ptr::NonNull) -> bool { + // CHECK: ret i1 false + let a: isize = unsafe { std::mem::transmute(x) }; + a == 0 +} + +// CHECK-LABEL: i1 @bool_ordering_is_ge(i1 +#[no_mangle] +pub fn bool_ordering_is_ge(x: bool) -> bool { + // CHECK: ret i1 true + let y: std::cmp::Ordering = unsafe { std::mem::transmute(x) }; + y.is_ge() +} + +// CHECK-LABEL: i1 @ordering_is_ge_then_transmute_to_bool(i8 +#[no_mangle] +pub fn ordering_is_ge_then_transmute_to_bool(x: std::cmp::Ordering) -> bool { + let r = x.is_ge(); + let _: bool = unsafe { std::mem::transmute(x) }; + r +} + +// CHECK-LABEL: i32 @normal_div(i32 +#[no_mangle] +pub fn normal_div(a: u32, b: u32) -> u32 { + // CHECK: call core::panicking::panic + a / b +} + +// CHECK-LABEL: i32 @div_transmute_nonzero(i32 +#[no_mangle] +pub fn div_transmute_nonzero(a: u32, b: std::num::NonZeroI32) -> u32 { + // CHECK-NOT: call core::panicking::panic + // CHECK: %[[R:.+]] = udiv i32 %a, %b + // CHECK-NEXT: ret i32 %[[R]] + // CHECK-NOT: call core::panicking::panic + let d: u32 = unsafe { std::mem::transmute(b) }; + a / d +} + +#[repr(i8)] +pub enum OneTwoThree { One = 1, Two = 2, Three = 3 } + +// CHECK-LABEL: i8 @ordering_transmute_onetwothree(i8 +#[no_mangle] +pub unsafe fn ordering_transmute_onetwothree(x: std::cmp::Ordering) -> OneTwoThree { + // CHECK: ret i8 1 + std::mem::transmute(x) +} + +// CHECK-LABEL: i8 @onetwothree_transmute_ordering(i8 +#[no_mangle] +pub unsafe fn onetwothree_transmute_ordering(x: OneTwoThree) -> std::cmp::Ordering { + // CHECK: ret i8 1 + std::mem::transmute(x) +} + +// CHECK-LABEL: i1 @char_is_negative(i32 +#[no_mangle] +pub fn char_is_negative(c: char) -> bool { + // CHECK: ret i1 false + let x: i32 = unsafe { std::mem::transmute(c) }; + x < 0 +}