Auto merge of #116010 - RalfJung:more-typed-immediates, r=oli-obk

bors · bors · commit 8da109609689 · 2023-09-21T14:02:55.000Z
interpret: more consistently use ImmTy in operators and casts

The diff in src/tools/miri/src/shims/x86/sse2.rs should hopefully suffice to explain why this is nicer. :)
diff --git a/src/concurrency/data_race.rs b/src/concurrency/data_race.rs
@@ -516,8 +516,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
         let old = this.allow_data_races_mut(|this| this.read_immediate(place))?;
 
         // Atomics wrap around on overflow.
-        let val = this.binary_op(op, &old, rhs)?;
-        let val = if neg { this.unary_op(mir::UnOp::Not, &val)? } else { val };
+        let val = this.wrapping_binary_op(op, &old, rhs)?;
+        let val = if neg { this.wrapping_unary_op(mir::UnOp::Not, &val)? } else { val };
         this.allow_data_races_mut(|this| this.write_immediate(*val, place))?;
 
         this.validate_atomic_rmw(place, atomic)?;
@@ -561,7 +561,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
 
         this.validate_overlapping_atomic(place)?;
         let old = this.allow_data_races_mut(|this| this.read_immediate(place))?;
-        let lt = this.binary_op(mir::BinOp::Lt, &old, &rhs)?.to_scalar().to_bool()?;
+        let lt = this.wrapping_binary_op(mir::BinOp::Lt, &old, &rhs)?.to_scalar().to_bool()?;
 
         let new_val = if min {
             if lt { &old } else { &rhs }
@@ -605,7 +605,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
         // Read as immediate for the sake of `binary_op()`
         let old = this.allow_data_races_mut(|this| this.read_immediate(place))?;
         // `binary_op` will bail if either of them is not a scalar.
-        let eq = this.binary_op(mir::BinOp::Eq, &old, expect_old)?;
+        let eq = this.wrapping_binary_op(mir::BinOp::Eq, &old, expect_old)?;
         // If the operation would succeed, but is "weak", fail some portion
         // of the time, based on `success_rate`.
         let success_rate = 1.0 - this.machine.cmpxchg_weak_failure_rate;
diff --git a/src/helpers.rs b/src/helpers.rs
@@ -1013,15 +1013,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     fn float_to_int_checked<F>(
         &self,
         f: F,
-        dest_ty: Ty<'tcx>,
+        cast_to: TyAndLayout<'tcx>,
         round: rustc_apfloat::Round,
-    ) -> Option<Scalar<Provenance>>
+    ) -> Option<ImmTy<'tcx, Provenance>>
     where
         F: rustc_apfloat::Float + Into<Scalar<Provenance>>,
     {
         let this = self.eval_context_ref();
 
-        match dest_ty.kind() {
+        let val = match cast_to.ty.kind() {
             // Unsigned
             ty::Uint(t) => {
                 let size = Integer::from_uint_ty(this, *t).size();
@@ -1033,11 +1033,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 ) {
                     // 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
+                    return 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))
+                    Scalar::from_uint(res.value, size)
                 }
             }
             // Signed
@@ -1051,20 +1051,22 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 ) {
                     // 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
+                    return 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))
+                    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:?}"
+                    "attempted float-to-int conversion with non-int output type {}",
+                    cast_to.ty,
                 ),
-        }
+        };
+        Some(ImmTy::from_scalar(val, cast_to))
     }
 
     /// Returns an integer type that is twice wide as `ty`
diff --git a/src/machine.rs b/src/machine.rs
@@ -998,7 +998,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
         bin_op: mir::BinOp,
         left: &ImmTy<'tcx, Provenance>,
         right: &ImmTy<'tcx, Provenance>,
-    ) -> InterpResult<'tcx, (Scalar<Provenance>, bool, Ty<'tcx>)> {
+    ) -> InterpResult<'tcx, (ImmTy<'tcx, Provenance>, bool)> {
         ecx.binary_ptr_op(bin_op, left, right)
     }
 
diff --git a/src/operator.rs b/src/operator.rs
@@ -1,6 +1,6 @@
 use log::trace;
 
-use rustc_middle::{mir, ty::Ty};
+use rustc_middle::mir;
 use rustc_target::abi::Size;
 
 use crate::*;
@@ -11,7 +11,7 @@ pub trait EvalContextExt<'tcx> {
         bin_op: mir::BinOp,
         left: &ImmTy<'tcx, Provenance>,
         right: &ImmTy<'tcx, Provenance>,
-    ) -> InterpResult<'tcx, (Scalar<Provenance>, bool, Ty<'tcx>)>;
+    ) -> InterpResult<'tcx, (ImmTy<'tcx, Provenance>, bool)>;
 }
 
 impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriInterpCx<'mir, 'tcx> {
@@ -20,7 +20,7 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriInterpCx<'mir, 'tcx> {
         bin_op: mir::BinOp,
         left: &ImmTy<'tcx, Provenance>,
         right: &ImmTy<'tcx, Provenance>,
-    ) -> InterpResult<'tcx, (Scalar<Provenance>, bool, Ty<'tcx>)> {
+    ) -> InterpResult<'tcx, (ImmTy<'tcx, Provenance>, bool)> {
         use rustc_middle::mir::BinOp::*;
 
         trace!("ptr_op: {:?} {:?} {:?}", *left, bin_op, *right);
@@ -50,7 +50,7 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriInterpCx<'mir, 'tcx> {
                     Ge => left >= right,
                     _ => bug!(),
                 };
-                (Scalar::from_bool(res), false, self.tcx.types.bool)
+                (ImmTy::from_bool(res, *self.tcx), false)
             }
 
             // Some more operations are possible with atomics.
@@ -65,12 +65,12 @@ impl<'mir, 'tcx> EvalContextExt<'tcx> for super::MiriInterpCx<'mir, 'tcx> {
                     right.to_scalar().to_target_usize(self)?,
                     self.machine.layouts.usize,
                 );
-                let (result, overflowing, _ty) =
+                let (result, overflowing) =
                     self.overflowing_binary_op(bin_op, &left, &right)?;
                 // Construct a new pointer with the provenance of `ptr` (the LHS).
                 let result_ptr =
-                    Pointer::new(ptr.provenance, Size::from_bytes(result.to_target_usize(self)?));
-                (Scalar::from_maybe_pointer(result_ptr, self), overflowing, left.layout.ty)
+                    Pointer::new(ptr.provenance, Size::from_bytes(result.to_scalar().to_target_usize(self)?));
+                (ImmTy::from_scalar(Scalar::from_maybe_pointer(result_ptr, self), left.layout), overflowing)
             }
 
             _ => span_bug!(self.cur_span(), "Invalid operator on pointers: {:?}", bin_op),
diff --git a/src/shims/intrinsics/mod.rs b/src/shims/intrinsics/mod.rs
@@ -89,10 +89,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let [left, right] = check_arg_count(args)?;
                 let left = this.read_immediate(left)?;
                 let right = this.read_immediate(right)?;
-                let (val, _overflowed, _ty) =
-                    this.overflowing_binary_op(mir::BinOp::Eq, &left, &right)?;
+                let val = this.wrapping_binary_op(mir::BinOp::Eq, &left, &right)?;
                 // We're type punning a bool as an u8 here.
-                this.write_scalar(val, dest)?;
+                this.write_scalar(val.to_scalar(), dest)?;
             }
             "const_allocate" => {
                 // For now, for compatibility with the run-time implementation of this, we just return null.
@@ -369,7 +368,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     ty::Float(FloatTy::F32) => {
                         let f = val.to_scalar().to_f32()?;
                         this
-                            .float_to_int_checked(f, dest.layout.ty, Round::TowardZero)
+                            .float_to_int_checked(f, dest.layout, Round::TowardZero)
                             .ok_or_else(|| {
                                 err_ub_format!(
                                     "`float_to_int_unchecked` intrinsic called on {f} which cannot be represented in target type `{:?}`",
@@ -380,7 +379,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     ty::Float(FloatTy::F64) => {
                         let f = val.to_scalar().to_f64()?;
                         this
-                            .float_to_int_checked(f, dest.layout.ty, Round::TowardZero)
+                            .float_to_int_checked(f, dest.layout, Round::TowardZero)
                             .ok_or_else(|| {
                                 err_ub_format!(
                                     "`float_to_int_unchecked` intrinsic called on {f} which cannot be represented in target type `{:?}`",
@@ -396,7 +395,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                         ),
                 };
 
-                this.write_scalar(res, dest)?;
+                this.write_immediate(*res, dest)?;
             }
 
             // Other
diff --git a/src/shims/intrinsics/simd.rs b/src/shims/intrinsics/simd.rs
@@ -60,7 +60,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     let op = this.read_immediate(&this.project_index(&op, i)?)?;
                     let dest = this.project_index(&dest, i)?;
                     let val = match which {
-                        Op::MirOp(mir_op) => this.unary_op(mir_op, &op)?.to_scalar(),
+                        Op::MirOp(mir_op) => this.wrapping_unary_op(mir_op, &op)?.to_scalar(),
                         Op::Abs => {
                             // Works for f32 and f64.
                             let ty::Float(float_ty) = op.layout.ty.kind() else {
@@ -177,7 +177,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     let dest = this.project_index(&dest, i)?;
                     let val = match which {
                         Op::MirOp(mir_op) => {
-                            let (val, overflowed, ty) = this.overflowing_binary_op(mir_op, &left, &right)?;
+                            let (val, overflowed) = this.overflowing_binary_op(mir_op, &left, &right)?;
                             if matches!(mir_op, BinOp::Shl | BinOp::Shr) {
                                 // Shifts have extra UB as SIMD operations that the MIR binop does not have.
                                 // See <https://github.com/rust-lang/rust/issues/91237>.
@@ -188,13 +188,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                             }
                             if matches!(mir_op, BinOp::Eq | BinOp::Ne | BinOp::Lt | BinOp::Le | BinOp::Gt | BinOp::Ge) {
                                 // Special handling for boolean-returning operations
-                                assert_eq!(ty, this.tcx.types.bool);
-                                let val = val.to_bool().unwrap();
+                                assert_eq!(val.layout.ty, this.tcx.types.bool);
+                                let val = val.to_scalar().to_bool().unwrap();
                                 bool_to_simd_element(val, dest.layout.size)
                             } else {
-                                assert_ne!(ty, this.tcx.types.bool);
-                                assert_eq!(ty, dest.layout.ty);
-                                val
+                                assert_ne!(val.layout.ty, this.tcx.types.bool);
+                                assert_eq!(val.layout.ty, dest.layout.ty);
+                                val.to_scalar()
                             }
                         }
                         Op::SaturatingOp(mir_op) => {
@@ -304,18 +304,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     let op = this.read_immediate(&this.project_index(&op, i)?)?;
                     res = match which {
                         Op::MirOp(mir_op) => {
-                            this.binary_op(mir_op, &res, &op)?
+                            this.wrapping_binary_op(mir_op, &res, &op)?
                         }
                         Op::MirOpBool(mir_op) => {
                             let op = imm_from_bool(simd_element_to_bool(op)?);
-                            this.binary_op(mir_op, &res, &op)?
+                            this.wrapping_binary_op(mir_op, &res, &op)?
                         }
                         Op::Max => {
                             if matches!(res.layout.ty.kind(), ty::Float(_)) {
                                 ImmTy::from_scalar(fmax_op(&res, &op)?, res.layout)
                             } else {
                                 // Just boring integers, so NaNs to worry about
-                                if this.binary_op(BinOp::Ge, &res, &op)?.to_scalar().to_bool()? {
+                                if this.wrapping_binary_op(BinOp::Ge, &res, &op)?.to_scalar().to_bool()? {
                                     res
                                 } else {
                                     op
@@ -327,7 +327,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                                 ImmTy::from_scalar(fmin_op(&res, &op)?, res.layout)
                             } else {
                                 // Just boring integers, so NaNs to worry about
-                                if this.binary_op(BinOp::Le, &res, &op)?.to_scalar().to_bool()? {
+                                if this.wrapping_binary_op(BinOp::Le, &res, &op)?.to_scalar().to_bool()? {
                                     res
                                 } else {
                                     op
@@ -356,7 +356,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let mut res = init;
                 for i in 0..op_len {
                     let op = this.read_immediate(&this.project_index(&op, i)?)?;
-                    res = this.binary_op(mir_op, &res, &op)?;
+                    res = this.wrapping_binary_op(mir_op, &res, &op)?;
                 }
                 this.write_immediate(*res, dest)?;
             }
@@ -441,17 +441,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                         // Int-to-(int|float): always safe
                         (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)?,
+                            this.int_to_int_or_float(&op, dest.layout)?,
                         // Float-to-float: always safe
                         (ty::Float(_), ty::Float(_)) if safe_cast || unsafe_cast =>
-                            this.float_to_float_or_int(&op, dest.layout.ty)?,
+                            this.float_to_float_or_int(&op, dest.layout)?,
                         // Float-to-int in safe mode
                         (ty::Float(_), ty::Int(_) | ty::Uint(_)) if safe_cast =>
-                            this.float_to_float_or_int(&op, dest.layout.ty)?,
+                            this.float_to_float_or_int(&op, dest.layout)?,
                         // Float-to-int in unchecked mode
                         (ty::Float(FloatTy::F32), ty::Int(_) | ty::Uint(_)) if unsafe_cast => {
                             let f = op.to_scalar().to_f32()?;
-                            this.float_to_int_checked(f, dest.layout.ty, Round::TowardZero)
+                            this.float_to_int_checked(f, dest.layout, Round::TowardZero)
                                 .ok_or_else(|| {
                                     err_ub_format!(
                                         "`simd_cast` intrinsic called on {f} which cannot be represented in target type `{:?}`",
@@ -462,7 +462,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                         }
                         (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)
+                            this.float_to_int_checked(f, dest.layout, Round::TowardZero)
                                 .ok_or_else(|| {
                                     err_ub_format!(
                                         "`simd_cast` intrinsic called on {f} which cannot be represented in target type `{:?}`",
@@ -473,12 +473,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                         }
                         // Ptr-to-ptr cast
                         (ty::RawPtr(..), ty::RawPtr(..)) if ptr_cast =>
-                            this.ptr_to_ptr(&op, dest.layout.ty)?,
+                            this.ptr_to_ptr(&op, dest.layout)?,
                         // Ptr/Int casts
                         (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::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)?,
                         // Error otherwise
                         _ =>
                             throw_unsup_format!(
@@ -487,7 +487,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                                 to_ty = dest.layout.ty,
                             ),
                     };
-                    this.write_immediate(val, &dest)?;
+                    this.write_immediate(*val, &dest)?;
                 }
             }
             "shuffle" => {
diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs
@@ -80,8 +80,8 @@ fn bin_op_float<'tcx, F: rustc_apfloat::Float>(
 ) -> InterpResult<'tcx, Scalar<Provenance>> {
     match which {
         FloatBinOp::Arith(which) => {
-            let (res, _overflow, _ty) = this.overflowing_binary_op(which, left, right)?;
-            Ok(res)
+            let res = this.wrapping_binary_op(which, left, right)?;
+            Ok(res.to_scalar())
         }
         FloatBinOp::Cmp(which) => {
             let left = left.to_scalar().to_float::<F>()?;
diff --git a/src/shims/x86/sse.rs b/src/shims/x86/sse.rs
diff --git a/src/shims/x86/sse2.rs b/src/shims/x86/sse2.rs

Original file line number	Diff line number	Diff line change
`@@ -998,7 +998,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {`
`998`	`998`	`bin_op: mir::BinOp,`
`999`	`999`	`left: &ImmTy<'tcx, Provenance>,`
`1000`	`1000`	`right: &ImmTy<'tcx, Provenance>,`
`1001`		`- ) -> InterpResult<'tcx, (Scalar<Provenance>, bool, Ty<'tcx>)> {`
	`1001`	`+ ) -> InterpResult<'tcx, (ImmTy<'tcx, Provenance>, bool)> {`
`1002`	`1002`	`ecx.binary_ptr_op(bin_op, left, right)`
`1003`	`1003`	`}`
`1004`	`1004`