From 852f4726710cf88e61cf4534f48c0d2b5348c2fd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 6 Apr 2024 22:01:22 +0200 Subject: [PATCH 1/7] MIRI_REPLACE_LIBRS_IF_NOT_TEST: also apply to crates.io crates --- src/tools/miri/cargo-miri/src/phases.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index 3f6c484a057d5..04f3b2918b55a 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/cargo-miri/src/phases.rs @@ -454,15 +454,10 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { continue; } // If the REPLACE_LIBRS hack is enabled and we are building a `lib.rs` file, and a - // `lib.miri.rs` file exists, then build that instead. We only consider relative paths - // as cargo uses those for files in the workspace; dependencies from crates.io get - // absolute paths. + // `lib.miri.rs` file exists, then build that instead. if replace_librs { let path = Path::new(&arg); - if path.is_relative() - && path.file_name().is_some_and(|f| f == "lib.rs") - && path.is_file() - { + if path.file_name().is_some_and(|f| f == "lib.rs") && path.is_file() { let miri_rs = Path::new(&arg).with_extension("miri.rs"); if miri_rs.is_file() { if verbose > 0 { From aff5f47dcef8e4616101a525cee7ecfd41183e6b Mon Sep 17 00:00:00 2001 From: klensy Date: Fri, 12 Apr 2024 17:54:35 +0300 Subject: [PATCH 2/7] remove dead code --- compiler/rustc_middle/src/util/common.rs | 30 ------------------------ 1 file changed, 30 deletions(-) diff --git a/compiler/rustc_middle/src/util/common.rs b/compiler/rustc_middle/src/util/common.rs index dd3a36c7bf853..0699e0b626946 100644 --- a/compiler/rustc_middle/src/util/common.rs +++ b/compiler/rustc_middle/src/util/common.rs @@ -1,8 +1,3 @@ -use rustc_data_structures::sync::Lock; - -use std::fmt::Debug; -use std::time::{Duration, Instant}; - #[cfg(test)] mod tests; @@ -26,31 +21,6 @@ pub fn to_readable_str(mut val: usize) -> String { groups.join("_") } -pub fn record_time(accu: &Lock, f: F) -> T -where - F: FnOnce() -> T, -{ - let start = Instant::now(); - let rv = f(); - let duration = start.elapsed(); - let mut accu = accu.lock(); - *accu += duration; - rv -} - -pub fn indent(op: F) -> R -where - R: Debug, - F: FnOnce() -> R, -{ - // Use in conjunction with the log post-processor like `src/etc/indenter` - // to make debug output more readable. - debug!(">>"); - let r = op(); - debug!("<< (Result = {:?})", r); - r -} - pub struct Indenter { _cannot_construct_outside_of_this_module: (), } From 474a0473b4a09f936ef2adf0ac9f22fc6b1488bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Fri, 12 Apr 2024 14:28:33 +0200 Subject: [PATCH 3/7] De-duplicate SSE2 sll/srl/sra code --- src/tools/miri/src/shims/x86/mod.rs | 80 ++++++++++++ src/tools/miri/src/shims/x86/sse2.rs | 176 +++------------------------ 2 files changed, 97 insertions(+), 159 deletions(-) diff --git a/src/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index 7b7921219e616..2a663d300a736 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/src/shims/x86/mod.rs @@ -468,6 +468,86 @@ fn unary_op_ps<'tcx>( Ok(()) } +enum ShiftOp { + /// Shift left, logically (shift in zeros) -- same as shift left, arithmetically + Left, + /// Shift right, logically (shift in zeros) + RightLogic, + /// Shift right, arithmetically (shift in sign) + RightArith, +} + +/// Shifts each element of `left` by a scalar amount. The shift amount +/// is determined by the lowest 64 bits of `right` (which is a 128-bit vector). +/// +/// For logic shifts, when right is larger than BITS - 1, zero is produced. +/// For arithmetic right-shifts, when right is larger than BITS - 1, the sign +/// bit is copied to remaining bits. +fn shift_simd_by_scalar<'tcx>( + this: &mut crate::MiriInterpCx<'_, 'tcx>, + left: &OpTy<'tcx, Provenance>, + right: &OpTy<'tcx, Provenance>, + which: ShiftOp, + dest: &MPlaceTy<'tcx, Provenance>, +) -> InterpResult<'tcx, ()> { + let (left, left_len) = this.operand_to_simd(left)?; + let (dest, dest_len) = this.mplace_to_simd(dest)?; + + assert_eq!(dest_len, left_len); + // `right` may have a different length, and we only care about its + // lowest 64bit anyway. + + // Get the 64-bit shift operand and convert it to the type expected + // by checked_{shl,shr} (u32). + // It is ok to saturate the value to u32::MAX because any value + // above BITS - 1 will produce the same result. + let shift = u32::try_from(extract_first_u64(this, right)?).unwrap_or(u32::MAX); + + for i in 0..dest_len { + let left = this.read_scalar(&this.project_index(&left, i)?)?; + let dest = this.project_index(&dest, i)?; + + let res = match which { + ShiftOp::Left => { + let left = left.to_uint(dest.layout.size)?; + let res = left.checked_shl(shift).unwrap_or(0); + // `truncate` is needed as left-shift can make the absolute value larger. + Scalar::from_uint(dest.layout.size.truncate(res), dest.layout.size) + } + ShiftOp::RightLogic => { + let left = left.to_uint(dest.layout.size)?; + let res = left.checked_shr(shift).unwrap_or(0); + // No `truncate` needed as right-shift can only make the absolute value smaller. + Scalar::from_uint(res, dest.layout.size) + } + ShiftOp::RightArith => { + let left = left.to_int(dest.layout.size)?; + // On overflow, copy the sign bit to the remaining bits + let res = left.checked_shr(shift).unwrap_or(left >> 127); + // No `truncate` needed as right-shift can only make the absolute value smaller. + Scalar::from_int(res, dest.layout.size) + } + }; + this.write_scalar(res, &dest)?; + } + + Ok(()) +} + +/// Takes a 128-bit vector, transmutes it to `[u64; 2]` and extracts +/// the first value. +fn extract_first_u64<'tcx>( + this: &crate::MiriInterpCx<'_, 'tcx>, + op: &OpTy<'tcx, Provenance>, +) -> InterpResult<'tcx, u64> { + // Transmute vector to `[u64; 2]` + let array_layout = this.layout_of(Ty::new_array(this.tcx.tcx, this.tcx.types.u64, 2))?; + let op = op.transmute(array_layout, this)?; + + // Get the first u64 from the array + this.read_scalar(&this.project_index(&op, 0)?)?.to_u64() +} + // Rounds the first element of `right` according to `rounding` // and copies the remaining elements from `left`. fn round_first<'tcx, F: rustc_apfloat::Float>( diff --git a/src/tools/miri/src/shims/x86/sse2.rs b/src/tools/miri/src/shims/x86/sse2.rs index eb2cc9d37c826..9db30d7ddca21 100644 --- a/src/tools/miri/src/shims/x86/sse2.rs +++ b/src/tools/miri/src/shims/x86/sse2.rs @@ -1,10 +1,11 @@ use rustc_apfloat::ieee::Double; -use rustc_middle::ty::layout::LayoutOf as _; -use rustc_middle::ty::Ty; use rustc_span::Symbol; use rustc_target::spec::abi::Abi; -use super::{bin_op_simd_float_all, bin_op_simd_float_first, convert_float_to_int, FloatBinOp}; +use super::{ + bin_op_simd_float_all, bin_op_simd_float_first, convert_float_to_int, shift_simd_by_scalar, + FloatBinOp, ShiftOp, +}; use crate::*; use shims::foreign_items::EmulateForeignItemResult; @@ -109,156 +110,27 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>: this.write_scalar(Scalar::from_u64(res.into()), &dest)?; } } - // Used to implement the _mm_{sll,srl,sra}_epi16 functions. - // Shifts 16-bit packed integers in left by the amount in right. - // Both operands are vectors of 16-bit integers. However, right is - // interpreted as a single 64-bit integer (remaining bits are ignored). - // For logic shifts, when right is larger than 15, zero is produced. - // For arithmetic shifts, when right is larger than 15, the sign bit + // Used to implement the _mm_{sll,srl,sra}_epi{16,32,64} functions + // (except _mm_sra_epi64, which is not available in SSE2). + // Shifts N-bit packed integers in left by the amount in right. + // Both operands are 128-bit vectors. However, right is interpreted as + // a single 64-bit integer (remaining bits are ignored). + // For logic shifts, when right is larger than N - 1, zero is produced. + // For arithmetic shifts, when right is larger than N - 1, the sign bit // is copied to remaining bits. - "psll.w" | "psrl.w" | "psra.w" => { + "psll.w" | "psrl.w" | "psra.w" | "psll.d" | "psrl.d" | "psra.d" | "psll.q" + | "psrl.q" => { let [left, right] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let (left, left_len) = this.operand_to_simd(left)?; - let (right, right_len) = this.operand_to_simd(right)?; - let (dest, dest_len) = this.mplace_to_simd(dest)?; - - assert_eq!(dest_len, left_len); - assert_eq!(dest_len, right_len); - - enum ShiftOp { - Sll, - Srl, - Sra, - } let which = match unprefixed_name { - "psll.w" => ShiftOp::Sll, - "psrl.w" => ShiftOp::Srl, - "psra.w" => ShiftOp::Sra, + "psll.w" | "psll.d" | "psll.q" => ShiftOp::Left, + "psrl.w" | "psrl.d" | "psrl.q" => ShiftOp::RightLogic, + "psra.w" | "psra.d" => ShiftOp::RightArith, _ => unreachable!(), }; - // Get the 64-bit shift operand and convert it to the type expected - // by checked_{shl,shr} (u32). - // It is ok to saturate the value to u32::MAX because any value - // above 15 will produce the same result. - let shift = extract_first_u64(this, &right)?.try_into().unwrap_or(u32::MAX); - - for i in 0..dest_len { - let left = this.read_scalar(&this.project_index(&left, i)?)?.to_u16()?; - let dest = this.project_index(&dest, i)?; - - let res = match which { - ShiftOp::Sll => left.checked_shl(shift).unwrap_or(0), - ShiftOp::Srl => left.checked_shr(shift).unwrap_or(0), - #[allow(clippy::cast_possible_wrap, clippy::cast_sign_loss)] - ShiftOp::Sra => { - // Convert u16 to i16 to use arithmetic shift - let left = left as i16; - // Copy the sign bit to the remaining bits - left.checked_shr(shift).unwrap_or(left >> 15) as u16 - } - }; - - this.write_scalar(Scalar::from_u16(res), &dest)?; - } - } - // Used to implement the _mm_{sll,srl,sra}_epi32 functions. - // 32-bit equivalent to the shift functions above. - "psll.d" | "psrl.d" | "psra.d" => { - let [left, right] = - this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - - let (left, left_len) = this.operand_to_simd(left)?; - let (right, right_len) = this.operand_to_simd(right)?; - let (dest, dest_len) = this.mplace_to_simd(dest)?; - - assert_eq!(dest_len, left_len); - assert_eq!(dest_len, right_len); - - enum ShiftOp { - Sll, - Srl, - Sra, - } - let which = match unprefixed_name { - "psll.d" => ShiftOp::Sll, - "psrl.d" => ShiftOp::Srl, - "psra.d" => ShiftOp::Sra, - _ => unreachable!(), - }; - - // Get the 64-bit shift operand and convert it to the type expected - // by checked_{shl,shr} (u32). - // It is ok to saturate the value to u32::MAX because any value - // above 31 will produce the same result. - let shift = extract_first_u64(this, &right)?.try_into().unwrap_or(u32::MAX); - - for i in 0..dest_len { - let left = this.read_scalar(&this.project_index(&left, i)?)?.to_u32()?; - let dest = this.project_index(&dest, i)?; - - let res = match which { - ShiftOp::Sll => left.checked_shl(shift).unwrap_or(0), - ShiftOp::Srl => left.checked_shr(shift).unwrap_or(0), - #[allow(clippy::cast_possible_wrap, clippy::cast_sign_loss)] - ShiftOp::Sra => { - // Convert u32 to i32 to use arithmetic shift - let left = left as i32; - // Copy the sign bit to the remaining bits - left.checked_shr(shift).unwrap_or(left >> 31) as u32 - } - }; - - this.write_scalar(Scalar::from_u32(res), &dest)?; - } - } - // Used to implement the _mm_{sll,srl}_epi64 functions. - // 64-bit equivalent to the shift functions above, except _mm_sra_epi64, - // which is not available in SSE2. - "psll.q" | "psrl.q" => { - let [left, right] = - this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - - let (left, left_len) = this.operand_to_simd(left)?; - let (right, right_len) = this.operand_to_simd(right)?; - let (dest, dest_len) = this.mplace_to_simd(dest)?; - - assert_eq!(dest_len, left_len); - assert_eq!(dest_len, right_len); - - enum ShiftOp { - Sll, - Srl, - } - let which = match unprefixed_name { - "psll.q" => ShiftOp::Sll, - "psrl.q" => ShiftOp::Srl, - _ => unreachable!(), - }; - - // Get the 64-bit shift operand and convert it to the type expected - // by checked_{shl,shr} (u32). - // It is ok to saturate the value to u32::MAX because any value - // above 63 will produce the same result. - let shift = this - .read_scalar(&this.project_index(&right, 0)?)? - .to_u64()? - .try_into() - .unwrap_or(u32::MAX); - - for i in 0..dest_len { - let left = this.read_scalar(&this.project_index(&left, i)?)?.to_u64()?; - let dest = this.project_index(&dest, i)?; - - let res = match which { - ShiftOp::Sll => left.checked_shl(shift).unwrap_or(0), - ShiftOp::Srl => left.checked_shr(shift).unwrap_or(0), - }; - - this.write_scalar(Scalar::from_u64(res), &dest)?; - } + shift_simd_by_scalar(this, left, right, which, dest)?; } // Used to implement the _mm_cvtps_epi32, _mm_cvttps_epi32, _mm_cvtpd_epi32 // and _mm_cvttpd_epi32 functions. @@ -585,17 +457,3 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>: Ok(EmulateForeignItemResult::NeedsJumping) } } - -/// Takes a 128-bit vector, transmutes it to `[u64; 2]` and extracts -/// the first value. -fn extract_first_u64<'tcx>( - this: &crate::MiriInterpCx<'_, 'tcx>, - op: &MPlaceTy<'tcx, Provenance>, -) -> InterpResult<'tcx, u64> { - // Transmute vector to `[u64; 2]` - let u64_array_layout = this.layout_of(Ty::new_array(this.tcx.tcx, this.tcx.types.u64, 2))?; - let op = op.transmute(u64_array_layout, this)?; - - // Get the first u64 from the array - this.read_scalar(&this.project_index(&op, 0)?)?.to_u64() -} From ed46e3ca65efd4111446cbc0c4d350f60f907eb0 Mon Sep 17 00:00:00 2001 From: Daniel Paoliello Date: Fri, 12 Apr 2024 16:28:19 -0700 Subject: [PATCH 4/7] Update backtrace submodule --- library/backtrace | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/backtrace b/library/backtrace index 6fa4b85b9962c..e15130618237e 160000 --- a/library/backtrace +++ b/library/backtrace @@ -1 +1 @@ -Subproject commit 6fa4b85b9962c3e1be8c2e5cc605cd078134152b +Subproject commit e15130618237eb3e2d4b622549f9647b4c1d9ca3 From c9889b02b92d28ab4b97bd3c57f4fd06e801697d Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Sat, 13 Apr 2024 06:17:07 +0000 Subject: [PATCH 5/7] fix coroutine witness doctest --- compiler/rustc_type_ir/src/ty_kind.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_type_ir/src/ty_kind.rs b/compiler/rustc_type_ir/src/ty_kind.rs index 56ae7ad9f741a..397e104512fd7 100644 --- a/compiler/rustc_type_ir/src/ty_kind.rs +++ b/compiler/rustc_type_ir/src/ty_kind.rs @@ -181,9 +181,9 @@ pub enum TyKind { /// Looking at the following example, the witness for this coroutine /// may end up as something like `for<'a> [Vec, &'a Vec]`: /// - /// ```ignore UNSOLVED (ask @compiler-errors, should this error? can we just swap the yields?) + /// ``` /// #![feature(coroutines)] - /// |a| { + /// static |a| { /// let x = &vec![3]; /// yield a; /// yield x[0]; From c6a4f810d926d00a176de40ae78288c145f3843d Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Sat, 13 Apr 2024 07:26:47 +0000 Subject: [PATCH 6/7] Replace a `DefiningOpaqueTypes::No` with `Yes` by asserting that one side of the comparison is a type variable. Thus there will never be an opaque type involved in a way that constrains its hidden type, as the other side of the comparison is always a generator witness type --- compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index da46ed076900d..2580179ce5bf3 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -561,9 +561,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Unify `interior` with `witness` and collect all the resulting obligations. let span = self.tcx.hir().body(body_id).value.span; + let ty::Infer(ty::InferTy::TyVar(_)) = interior.kind() else { + span_bug!(span, "coroutine interior witness not infer: {:?}", interior.kind()) + }; let ok = self .at(&self.misc(span), self.param_env) - .eq(DefineOpaqueTypes::No, interior, witness) + // Will never define opaque types, as all we do is instantiate a type variable. + .eq(DefineOpaqueTypes::Yes, interior, witness) .expect("Failed to unify coroutine interior type"); let mut obligations = ok.obligations; From 7085ff2f6023800ea34165d49df8ff4d2b6b4bc4 Mon Sep 17 00:00:00 2001 From: klensy Date: Fri, 12 Apr 2024 18:04:22 +0300 Subject: [PATCH 7/7] remove some ancient debug output, looks unused? --- compiler/rustc_hir_analysis/src/check/check.rs | 1 - compiler/rustc_hir_analysis/src/check/mod.rs | 1 - compiler/rustc_hir_analysis/src/lib.rs | 1 - compiler/rustc_middle/src/util/common.rs | 15 --------------- 4 files changed, 18 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 961b643fa2510..216b89fd4f150 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -505,7 +505,6 @@ fn check_static_linkage(tcx: TyCtxt<'_>, def_id: LocalDefId) { } pub(crate) fn check_item_type(tcx: TyCtxt<'_>, def_id: LocalDefId) { - let _indenter = indenter(); match tcx.def_kind(def_id) { DefKind::Static { .. } => { tcx.ensure().typeck(def_id); diff --git a/compiler/rustc_hir_analysis/src/check/mod.rs b/compiler/rustc_hir_analysis/src/check/mod.rs index 938c0a19e338f..024a0433b8cf6 100644 --- a/compiler/rustc_hir_analysis/src/check/mod.rs +++ b/compiler/rustc_hir_analysis/src/check/mod.rs @@ -103,7 +103,6 @@ use rustc_trait_selection::traits::ObligationCtxt; use crate::errors; use crate::require_c_abi_if_c_variadic; -use crate::util::common::indenter; use self::compare_impl_item::collect_return_position_impl_trait_in_trait_tys; use self::region::region_scope_tree; diff --git a/compiler/rustc_hir_analysis/src/lib.rs b/compiler/rustc_hir_analysis/src/lib.rs index 5a374fa5e04c0..c374f9762d69b 100644 --- a/compiler/rustc_hir_analysis/src/lib.rs +++ b/compiler/rustc_hir_analysis/src/lib.rs @@ -103,7 +103,6 @@ use rustc_middle::middle; use rustc_middle::mir::interpret::GlobalId; use rustc_middle::query::Providers; use rustc_middle::ty::{self, Ty, TyCtxt}; -use rustc_middle::util; use rustc_session::parse::feature_err; use rustc_span::{symbol::sym, Span}; use rustc_target::spec::abi::Abi; diff --git a/compiler/rustc_middle/src/util/common.rs b/compiler/rustc_middle/src/util/common.rs index 0699e0b626946..2038d3f84484e 100644 --- a/compiler/rustc_middle/src/util/common.rs +++ b/compiler/rustc_middle/src/util/common.rs @@ -21,21 +21,6 @@ pub fn to_readable_str(mut val: usize) -> String { groups.join("_") } -pub struct Indenter { - _cannot_construct_outside_of_this_module: (), -} - -impl Drop for Indenter { - fn drop(&mut self) { - debug!("<<"); - } -} - -pub fn indenter() -> Indenter { - debug!(">>"); - Indenter { _cannot_construct_outside_of_this_module: () } -} - // const wrapper for `if let Some((_, tail)) = name.rsplit_once(':') { tail } else { name }` pub const fn c_name(name: &'static str) -> &'static str { // FIXME Simplify the implementation once more `str` methods get const-stable.