Auto merge of #3025 - eduardosm:float-to-int, r=RalfJung

Add checked float-to-int helper function

As discussed in https://github.com/rust-lang/miri/pull/2989#discussion_r1287252367

src/tools/miri/src/helpers.rs

@@ -13,11 +13,11 @@ use rustc_index::IndexVec;
 use rustc_middle::mir;
 use rustc_middle::ty::{
     self,
-    layout::{LayoutOf, TyAndLayout},
+    layout::{IntegerExt as _, LayoutOf, TyAndLayout},
-    List, TyCtxt,
+    List, Ty, TyCtxt,
 };
 use rustc_span::{def_id::CrateNum, sym, Span, Symbol};
-use rustc_target::abi::{Align, FieldIdx, FieldsShape, Size, Variants};
+use rustc_target::abi::{Align, FieldIdx, FieldsShape, Integer, Size, Variants};
 use rustc_target::spec::abi::Abi;
 
 use rand::RngCore;
@@ -1011,6 +1011,65 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             None => tcx.item_name(def_id),
         }
     }
+
+    /// Converts `f` to integer type `dest_ty` after rounding with mode `round`.
+    /// Returns `None` if `f` is NaN or out of range.
+    fn float_to_int_checked<F>(
+        &self,
+        f: F,
+        dest_ty: Ty<'tcx>,
+        round: rustc_apfloat::Round,
+    ) -> Option<Scalar<Provenance>>
+    where
+        F: rustc_apfloat::Float + Into<Scalar<Provenance>>,
+    {
+        let this = self.eval_context_ref();
+
+        match dest_ty.kind() {
+            // Unsigned
+            ty::Uint(t) => {
+                let size = Integer::from_uint_ty(this, *t).size();
+                let res = f.to_u128_r(size.bits_usize(), round, &mut false);
+                if res.status.intersects(
+                    rustc_apfloat::Status::INVALID_OP
+                        | rustc_apfloat::Status::OVERFLOW
+                        | rustc_apfloat::Status::UNDERFLOW,
+                ) {
+                    // Floating point value is NaN (flagged with INVALID_OP) or outside the range
+                    // of values of the integer type (flagged with OVERFLOW or UNDERFLOW).
+                    None
+                } else {
+                    // Floating point value can be represented by the integer type after rounding.
+                    // The INEXACT flag is ignored on purpose to allow rounding.
+                    Some(Scalar::from_uint(res.value, size))
+                }
+            }
+            // Signed
+            ty::Int(t) => {
+                let size = Integer::from_int_ty(this, *t).size();
+                let res = f.to_i128_r(size.bits_usize(), round, &mut false);
+                if res.status.intersects(
+                    rustc_apfloat::Status::INVALID_OP
+                        | rustc_apfloat::Status::OVERFLOW
+                        | rustc_apfloat::Status::UNDERFLOW,
+                ) {
+                    // Floating point value is NaN (flagged with INVALID_OP) or outside the range
+                    // of values of the integer type (flagged with OVERFLOW or UNDERFLOW).
+                    None
+                } else {
+                    // Floating point value can be represented by the integer type after rounding.
+                    // The INEXACT flag is ignored on purpose to allow rounding.
+                    Some(Scalar::from_int(res.value, size))
+                }
+            }
+            // Nothing else
+            _ =>
+                span_bug!(
+                    this.cur_span(),
+                    "attempted float-to-int conversion with non-int output type {dest_ty:?}"
+                ),
+        }
+    }
 }
 
 impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {

src/tools/miri/src/shims/intrinsics/mod.rs

@@ -6,12 +6,12 @@ use std::iter;
 use log::trace;
 
 use rustc_apfloat::{Float, Round};
-use rustc_middle::ty::layout::{IntegerExt, LayoutOf};
+use rustc_middle::ty::layout::LayoutOf;
 use rustc_middle::{
     mir,
-    ty::{self, FloatTy, Ty},
+    ty::{self, FloatTy},
 };
-use rustc_target::abi::{Integer, Size};
+use rustc_target::abi::Size;
 
 use crate::*;
 use atomic::EvalContextExt as _;
@@ -356,10 +356,28 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let val = this.read_immediate(val)?;
 
                 let res = match val.layout.ty.kind() {
-                    ty::Float(FloatTy::F32) =>
+                    ty::Float(FloatTy::F32) => {
-                        this.float_to_int_unchecked(val.to_scalar().to_f32()?, dest.layout.ty)?,
+                        let f = val.to_scalar().to_f32()?;
-                    ty::Float(FloatTy::F64) =>
+                        this
-                        this.float_to_int_unchecked(val.to_scalar().to_f64()?, dest.layout.ty)?,
+                            .float_to_int_checked(f, dest.layout.ty, Round::TowardZero)
+                            .ok_or_else(|| {
+                                err_ub_format!(
+                                    "`float_to_int_unchecked` intrinsic called on {f} which cannot be represented in target type `{:?}`",
+                                    dest.layout.ty
+                                )
+                            })?
+                    }
+                    ty::Float(FloatTy::F64) => {
+                        let f = val.to_scalar().to_f64()?;
+                        this
+                            .float_to_int_checked(f, dest.layout.ty, Round::TowardZero)
+                            .ok_or_else(|| {
+                                err_ub_format!(
+                                    "`float_to_int_unchecked` intrinsic called on {f} which cannot be represented in target type `{:?}`",
+                                    dest.layout.ty
+                                )
+                            })?
+                    }
                     _ =>
                         span_bug!(
                             this.cur_span(),
@@ -383,57 +401,4 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
         Ok(())
     }
-
-    fn float_to_int_unchecked<F>(
-        &self,
-        f: F,
-        dest_ty: Ty<'tcx>,
-    ) -> InterpResult<'tcx, Scalar<Provenance>>
-    where
-        F: Float + Into<Scalar<Provenance>>,
-    {
-        let this = self.eval_context_ref();
-
-        // Step 1: cut off the fractional part of `f`. The result of this is
-        // guaranteed to be precisely representable in IEEE floats.
-        let f = f.round_to_integral(Round::TowardZero).value;
-
-        // Step 2: Cast the truncated float to the target integer type and see if we lose any information in this step.
-        Ok(match dest_ty.kind() {
-            // Unsigned
-            ty::Uint(t) => {
-                let size = Integer::from_uint_ty(this, *t).size();
-                let res = f.to_u128(size.bits_usize());
-                if res.status.is_empty() {
-                    // No status flags means there was no further rounding or other loss of precision.
-                    Scalar::from_uint(res.value, size)
-                } else {
-                    // `f` was not representable in this integer type.
-                    throw_ub_format!(
-                        "`float_to_int_unchecked` intrinsic called on {f} which cannot be represented in target type `{dest_ty:?}`",
-                    );
-                }
-            }
-            // Signed
-            ty::Int(t) => {
-                let size = Integer::from_int_ty(this, *t).size();
-                let res = f.to_i128(size.bits_usize());
-                if res.status.is_empty() {
-                    // No status flags means there was no further rounding or other loss of precision.
-                    Scalar::from_int(res.value, size)
-                } else {
-                    // `f` was not representable in this integer type.
-                    throw_ub_format!(
-                        "`float_to_int_unchecked` intrinsic called on {f} which cannot be represented in target type `{dest_ty:?}`",
-                    );
-                }
-            }
-            // Nothing else
-            _ =>
-                span_bug!(
-                    this.cur_span(),
-                    "`float_to_int_unchecked` called with non-int output type {dest_ty:?}"
-                ),
-        })
-    }
 }

src/tools/miri/src/shims/intrinsics/simd.rs

@@ -1,4 +1,4 @@
-use rustc_apfloat::Float;
+use rustc_apfloat::{Float, Round};
 use rustc_middle::ty::layout::{HasParamEnv, LayoutOf};
 use rustc_middle::{mir, ty, ty::FloatTy};
 use rustc_target::abi::{Endian, HasDataLayout, Size};
@@ -420,7 +420,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     }
                 }
             }
-            #[rustfmt::skip]
             "cast" | "as" | "cast_ptr" | "expose_addr" | "from_exposed_addr" => {
                 let [op] = check_arg_count(args)?;
                 let (op, op_len) = this.operand_to_simd(op)?;
@@ -440,7 +439,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
                     let val = match (op.layout.ty.kind(), dest.layout.ty.kind()) {
                         // Int-to-(int|float): always safe
-                        (ty::Int(_) | ty::Uint(_), ty::Int(_) | ty::Uint(_) | ty::Float(_)) if safe_cast || unsafe_cast =>
+                        (ty::Int(_) | ty::Uint(_), ty::Int(_) | ty::Uint(_) | ty::Float(_))
+                            if safe_cast || unsafe_cast =>
                             this.int_to_int_or_float(&op, dest.layout.ty)?,
                         // Float-to-float: always safe
                         (ty::Float(_), ty::Float(_)) if safe_cast || unsafe_cast =>
@@ -449,21 +449,36 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                         (ty::Float(_), ty::Int(_) | ty::Uint(_)) if safe_cast =>
                             this.float_to_float_or_int(&op, dest.layout.ty)?,
                         // Float-to-int in unchecked mode
-                        (ty::Float(FloatTy::F32), ty::Int(_) | ty::Uint(_)) if unsafe_cast =>
+                        (ty::Float(FloatTy::F32), ty::Int(_) | ty::Uint(_)) if unsafe_cast => {
-                            this.float_to_int_unchecked(op.to_scalar().to_f32()?, dest.layout.ty)?.into(),
+                            let f = op.to_scalar().to_f32()?;
-                        (ty::Float(FloatTy::F64), ty::Int(_) | ty::Uint(_)) if unsafe_cast =>
+                            this.float_to_int_checked(f, dest.layout.ty, Round::TowardZero)
-                            this.float_to_int_unchecked(op.to_scalar().to_f64()?, dest.layout.ty)?.into(),
+                                .ok_or_else(|| {
+                                    err_ub_format!(
+                                        "`simd_cast` intrinsic called on {f} which cannot be represented in target type `{:?}`",
+                                        dest.layout.ty
+                                    )
+                                })?
+                                .into()
+                        }
+                        (ty::Float(FloatTy::F64), ty::Int(_) | ty::Uint(_)) if unsafe_cast => {
+                            let f = op.to_scalar().to_f64()?;
+                            this.float_to_int_checked(f, dest.layout.ty, Round::TowardZero)
+                                .ok_or_else(|| {
+                                    err_ub_format!(
+                                        "`simd_cast` intrinsic called on {f} which cannot be represented in target type `{:?}`",
+                                        dest.layout.ty
+                                    )
+                                })?
+                                .into()
+                        }
                         // Ptr-to-ptr cast
-                        (ty::RawPtr(..), ty::RawPtr(..)) if ptr_cast => {
+                        (ty::RawPtr(..), ty::RawPtr(..)) if ptr_cast =>
-                            this.ptr_to_ptr(&op, dest.layout.ty)?
+                            this.ptr_to_ptr(&op, dest.layout.ty)?,
-                        }
                         // Ptr/Int casts
-                        (ty::RawPtr(..), ty::Int(_) | ty::Uint(_)) if expose_cast => {
+                        (ty::RawPtr(..), ty::Int(_) | ty::Uint(_)) if expose_cast =>
-                            this.pointer_expose_address_cast(&op, dest.layout.ty)?
+                            this.pointer_expose_address_cast(&op, dest.layout.ty)?,
-                        }
+                        (ty::Int(_) | ty::Uint(_), ty::RawPtr(..)) if from_exposed_cast =>
-                        (ty::Int(_) | ty::Uint(_), ty::RawPtr(..)) if from_exposed_cast => {
+                            this.pointer_from_exposed_address_cast(&op, dest.layout.ty)?,
-                            this.pointer_from_exposed_address_cast(&op, dest.layout.ty)?
-                        }
                         // Error otherwise
                         _ =>
                             throw_unsup_format!(

src/tools/miri/src/shims/x86/sse.rs

@@ -195,24 +195,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     _ => unreachable!(),
                 };
 
-                let mut exact = false;
+                let res = this.float_to_int_checked(op, dest.layout.ty, rnd).unwrap_or_else(|| {
-                let cvt = op.to_i128_r(32, rnd, &mut exact);
+                    // Fallback to minimum acording to SSE semantics.
-                let res = if cvt.status.intersects(
+                    Scalar::from_i32(i32::MIN)
-                    rustc_apfloat::Status::INVALID_OP
+                });
-                        | rustc_apfloat::Status::OVERFLOW
-                        | rustc_apfloat::Status::UNDERFLOW,
-                ) {
-                    // Input is NaN (flagged with INVALID_OP) or does not fit
-                    // in an i32 (flagged with OVERFLOW or UNDERFLOW), fallback
-                    // to minimum acording to SSE semantics. The INEXACT flag
-                    // is ignored on purpose because rounding can happen during
-                    // float-to-int conversion.
-                    i32::MIN
-                } else {
-                    i32::try_from(cvt.value).unwrap()
-                };
 
-                this.write_scalar(Scalar::from_i32(res), dest)?;
+                this.write_scalar(res, dest)?;
             }
             // Use to implement _mm_cvtss_si64 and _mm_cvttss_si64.
             // Converts the first component of `op` from f32 to i64.
@@ -232,24 +220,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     _ => unreachable!(),
                 };
 
-                let mut exact = false;
+                let res = this.float_to_int_checked(op, dest.layout.ty, rnd).unwrap_or_else(|| {
-                let cvt = op.to_i128_r(64, rnd, &mut exact);
+                    // Fallback to minimum acording to SSE semantics.
-                let res = if cvt.status.intersects(
+                    Scalar::from_i64(i64::MIN)
-                    rustc_apfloat::Status::INVALID_OP
+                });
-                        | rustc_apfloat::Status::OVERFLOW
-                        | rustc_apfloat::Status::UNDERFLOW,
-                ) {
-                    // Input is NaN (flagged with INVALID_OP) or does not fit
-                    // in an i64 (flagged with OVERFLOW or UNDERFLOW), fallback
-                    // to minimum acording to SSE semantics. The INEXACT flag
-                    // is ignored on purpose because rounding can happen during
-                    // float-to-int conversion.
-                    i64::MIN
-                } else {
-                    i64::try_from(cvt.value).unwrap()
-                };
 
-                this.write_scalar(Scalar::from_i64(res), dest)?;
+                this.write_scalar(res, dest)?;
             }
             // Used to implement the _mm_cvtsi32_ss function.
             // Converts `right` from i32 to f32. Returns a SIMD vector with

src/tools/miri/tests/fail/intrinsics/float_to_int_64_neg.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: `float_to_int_unchecked` intrinsic called on -1 which cannot be represented in target type `u128`
+error: Undefined Behavior: `float_to_int_unchecked` intrinsic called on -1.0000000000000999 which cannot be represented in target type `u128`
   --> $DIR/float_to_int_64_neg.rs:LL:CC
    |
 LL |         float_to_int_unchecked::<f64, u128>(-1.0000000000001f64);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `float_to_int_unchecked` intrinsic called on -1 which cannot be represented in target type `u128`
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `float_to_int_unchecked` intrinsic called on -1.0000000000000999 which cannot be represented in target type `u128`
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

src/tools/miri/tests/fail/intrinsics/simd-float-to-int.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: `float_to_int_unchecked` intrinsic called on 3.40282347E+38 which cannot be represented in target type `i32`
+error: Undefined Behavior: `simd_cast` intrinsic called on 3.40282347E+38 which cannot be represented in target type `i32`
   --> $DIR/simd-float-to-int.rs:LL:CC
    |
 LL |         let _x: i32x2 = f32x2::from_array([f32::MAX, f32::MIN]).to_int_unchecked();
-   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `float_to_int_unchecked` intrinsic called on 3.40282347E+38 which cannot be represented in target type `i32`
+   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `simd_cast` intrinsic called on 3.40282347E+38 which cannot be represented in target type `i32`
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information