From 6542c2110e19e3ac9f913db6b8e8dc704233e5c5 Mon Sep 17 00:00:00 2001 From: DianQK Date: Sat, 2 Nov 2024 13:50:05 +0800 Subject: [PATCH 1/4] Do not use `for_each_assignment_mut` to iterate over assignment statements `for_each_assignment_mut` can skip assignment statements with side effects, which can result in some assignment statements retrieving outdated value. For example, it may skip a dereference assignment statement. --- compiler/rustc_mir_transform/src/gvn.rs | 77 +++++++++---------- compiler/rustc_mir_transform/src/ssa.rs | 37 --------- ..._simplification.hello.GVN.panic-abort.diff | 3 +- ...simplification.hello.GVN.panic-unwind.diff | 3 +- 4 files changed, 41 insertions(+), 79 deletions(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 274eea9563fe8..c84a4718d114f 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -3,14 +3,16 @@ //! MIR may contain repeated and/or redundant computations. The objective of this pass is to detect //! such redundancies and re-use the already-computed result when possible. //! -//! In a first pass, we compute a symbolic representation of values that are assigned to SSA -//! locals. This symbolic representation is defined by the `Value` enum. Each produced instance of -//! `Value` is interned as a `VnIndex`, which allows us to cheaply compute identical values. -//! //! From those assignments, we construct a mapping `VnIndex -> Vec<(Local, Location)>` of available //! values, the locals in which they are stored, and the assignment location. //! -//! In a second pass, we traverse all (non SSA) assignments `x = rvalue` and operands. For each +//! We traverse all assignments `x = rvalue` and operands. +//! +//! For each SSA one, we compute a symbolic representation of values that are assigned to SSA +//! locals. This symbolic representation is defined by the `Value` enum. Each produced instance of +//! `Value` is interned as a `VnIndex`, which allows us to cheaply compute identical values. +//! +//! For each non-SSA //! one, we compute the `VnIndex` of the rvalue. If this `VnIndex` is associated to a constant, we //! replace the rvalue/operand by that constant. Otherwise, if there is an SSA local `y` //! associated to this `VnIndex`, and if its definition location strictly dominates the assignment @@ -107,7 +109,7 @@ use rustc_span::def_id::DefId; use smallvec::SmallVec; use tracing::{debug, instrument, trace}; -use crate::ssa::{AssignedValue, SsaLocals}; +use crate::ssa::SsaLocals; pub(super) struct GVN; @@ -126,31 +128,11 @@ impl<'tcx> crate::MirPass<'tcx> for GVN { let dominators = body.basic_blocks.dominators().clone(); let mut state = VnState::new(tcx, body, param_env, &ssa, dominators, &body.local_decls); - ssa.for_each_assignment_mut( - body.basic_blocks.as_mut_preserves_cfg(), - |local, value, location| { - let value = match value { - // We do not know anything of this assigned value. - AssignedValue::Arg | AssignedValue::Terminator => None, - // Try to get some insight. - AssignedValue::Rvalue(rvalue) => { - let value = state.simplify_rvalue(rvalue, location); - // FIXME(#112651) `rvalue` may have a subtype to `local`. We can only mark - // `local` as reusable if we have an exact type match. - if state.local_decls[local].ty != rvalue.ty(state.local_decls, tcx) { - return; - } - value - } - }; - // `next_opaque` is `Some`, so `new_opaque` must return `Some`. - let value = value.or_else(|| state.new_opaque()).unwrap(); - state.assign(local, value); - }, - ); - // Stop creating opaques during replacement as it is useless. - state.next_opaque = None; + for local in body.args_iter().filter(|&local| ssa.is_ssa(local)) { + let opaque = state.new_opaque().unwrap(); + state.assign(local, opaque); + } let reverse_postorder = body.basic_blocks.reverse_postorder().to_vec(); for bb in reverse_postorder { @@ -247,7 +229,6 @@ struct VnState<'body, 'tcx> { locals: IndexVec>, /// Locals that are assigned that value. // This vector does not hold all the values of `VnIndex` that we create. - // It stops at the largest value created in the first phase of collecting assignments. rev_locals: IndexVec>, values: FxIndexSet>, /// Values evaluated as constants if possible. @@ -339,6 +320,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { /// Record that `local` is assigned `value`. `local` must be SSA. #[instrument(level = "trace", skip(self))] fn assign(&mut self, local: Local, value: VnIndex) { + debug_assert!(self.ssa.is_ssa(local)); self.locals[local] = Some(value); // Only register the value if its type is `Sized`, as we will emit copies of it. @@ -1633,15 +1615,19 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> { if let StatementKind::Assign(box (ref mut lhs, ref mut rvalue)) = stmt.kind { self.simplify_place_projection(lhs, location); - // Do not try to simplify a constant, it's already in canonical shape. - if matches!(rvalue, Rvalue::Use(Operand::Constant(_))) { - return; - } - - let value = lhs - .as_local() - .and_then(|local| self.locals[local]) - .or_else(|| self.simplify_rvalue(rvalue, location)); + let value = self.simplify_rvalue(rvalue, location); + let value = if let Some(local) = lhs.as_local() + && self.ssa.is_ssa(local) + // FIXME(#112651) `rvalue` may have a subtype to `local`. We can only mark + // `local` as reusable if we have an exact type match. + && self.local_decls[local].ty == rvalue.ty(self.local_decls, self.tcx) + { + let value = value.or_else(|| self.new_opaque()).unwrap(); + self.assign(local, value); + Some(value) + } else { + value + }; let Some(value) = value else { return }; if let Some(const_) = self.try_as_constant(value) { @@ -1657,6 +1643,17 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> { } self.super_statement(stmt, location); } + + fn visit_terminator(&mut self, terminator: &mut Terminator<'tcx>, location: Location) { + if let Terminator { kind: TerminatorKind::Call { destination, .. }, .. } = terminator + && let Some(local) = destination.as_local() + && self.ssa.is_ssa(local) + { + let opaque = self.new_opaque().unwrap(); + self.assign(local, opaque); + } + self.super_terminator(terminator, location); + } } struct StorageRemover<'tcx> { diff --git a/compiler/rustc_mir_transform/src/ssa.rs b/compiler/rustc_mir_transform/src/ssa.rs index 84df666e34a72..72482f9b0fe7c 100644 --- a/compiler/rustc_mir_transform/src/ssa.rs +++ b/compiler/rustc_mir_transform/src/ssa.rs @@ -32,12 +32,6 @@ pub(super) struct SsaLocals { borrowed_locals: BitSet, } -pub(super) enum AssignedValue<'a, 'tcx> { - Arg, - Rvalue(&'a mut Rvalue<'tcx>), - Terminator, -} - impl SsaLocals { pub(super) fn new<'tcx>( tcx: TyCtxt<'tcx>, @@ -152,37 +146,6 @@ impl SsaLocals { }) } - pub(super) fn for_each_assignment_mut<'tcx>( - &self, - basic_blocks: &mut IndexSlice>, - mut f: impl FnMut(Local, AssignedValue<'_, 'tcx>, Location), - ) { - for &local in &self.assignment_order { - match self.assignments[local] { - Set1::One(DefLocation::Argument) => f(local, AssignedValue::Arg, Location { - block: START_BLOCK, - statement_index: 0, - }), - Set1::One(DefLocation::Assignment(loc)) => { - let bb = &mut basic_blocks[loc.block]; - // `loc` must point to a direct assignment to `local`. - let stmt = &mut bb.statements[loc.statement_index]; - let StatementKind::Assign(box (target, ref mut rvalue)) = stmt.kind else { - bug!() - }; - assert_eq!(target.as_local(), Some(local)); - f(local, AssignedValue::Rvalue(rvalue), loc) - } - Set1::One(DefLocation::CallReturn { call, .. }) => { - let bb = &mut basic_blocks[call]; - let loc = Location { block: call, statement_index: bb.statements.len() }; - f(local, AssignedValue::Terminator, loc) - } - _ => {} - } - } - } - /// Compute the equivalence classes for locals, based on copy statements. /// /// The returned vector maps each local to the one it copies. In the following case: diff --git a/tests/mir-opt/const_prop/control_flow_simplification.hello.GVN.panic-abort.diff b/tests/mir-opt/const_prop/control_flow_simplification.hello.GVN.panic-abort.diff index 886f9a68dd9d1..417406de39b61 100644 --- a/tests/mir-opt/const_prop/control_flow_simplification.hello.GVN.panic-abort.diff +++ b/tests/mir-opt/const_prop/control_flow_simplification.hello.GVN.panic-abort.diff @@ -8,8 +8,9 @@ bb0: { StorageLive(_1); - _1 = const ::NEEDS; +- _1 = const ::NEEDS; - switchInt(move _1) -> [0: bb2, otherwise: bb1]; ++ _1 = const false; + switchInt(const false) -> [0: bb2, otherwise: bb1]; } diff --git a/tests/mir-opt/const_prop/control_flow_simplification.hello.GVN.panic-unwind.diff b/tests/mir-opt/const_prop/control_flow_simplification.hello.GVN.panic-unwind.diff index cc53b213397d8..63ba2c6865f0a 100644 --- a/tests/mir-opt/const_prop/control_flow_simplification.hello.GVN.panic-unwind.diff +++ b/tests/mir-opt/const_prop/control_flow_simplification.hello.GVN.panic-unwind.diff @@ -8,8 +8,9 @@ bb0: { StorageLive(_1); - _1 = const ::NEEDS; +- _1 = const ::NEEDS; - switchInt(move _1) -> [0: bb2, otherwise: bb1]; ++ _1 = const false; + switchInt(const false) -> [0: bb2, otherwise: bb1]; } From a60983c1e2aa6aeb47fda98ffe6dc1389d21ca08 Mon Sep 17 00:00:00 2001 From: DianQK Date: Sat, 2 Nov 2024 20:11:41 +0800 Subject: [PATCH 2/4] `next_opaque` is no longer an `Option` --- compiler/rustc_mir_transform/src/gvn.rs | 75 ++++++++++++------------- 1 file changed, 35 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index c84a4718d114f..618001b1d7594 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -130,7 +130,7 @@ impl<'tcx> crate::MirPass<'tcx> for GVN { let mut state = VnState::new(tcx, body, param_env, &ssa, dominators, &body.local_decls); for local in body.args_iter().filter(|&local| ssa.is_ssa(local)) { - let opaque = state.new_opaque().unwrap(); + let opaque = state.new_opaque(); state.assign(local, opaque); } @@ -234,8 +234,7 @@ struct VnState<'body, 'tcx> { /// Values evaluated as constants if possible. evaluated: IndexVec>>, /// Counter to generate different values. - /// This is an option to stop creating opaques during replacement. - next_opaque: Option, + next_opaque: usize, /// Cache the value of the `unsized_locals` features, to avoid fetching it repeatedly in a loop. feature_unsized_locals: bool, ssa: &'body SsaLocals, @@ -268,7 +267,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { rev_locals: IndexVec::with_capacity(num_values), values: FxIndexSet::with_capacity_and_hasher(num_values, Default::default()), evaluated: IndexVec::with_capacity(num_values), - next_opaque: Some(1), + next_opaque: 1, feature_unsized_locals: tcx.features().unsized_locals(), ssa, dominators, @@ -285,32 +284,31 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { let evaluated = self.eval_to_const(index); let _index = self.evaluated.push(evaluated); debug_assert_eq!(index, _index); - // No need to push to `rev_locals` if we finished listing assignments. - if self.next_opaque.is_some() { - let _index = self.rev_locals.push(SmallVec::new()); - debug_assert_eq!(index, _index); - } + let _index = self.rev_locals.push(SmallVec::new()); + debug_assert_eq!(index, _index); } index } + fn next_opaque(&mut self) -> usize { + let next_opaque = self.next_opaque; + self.next_opaque += 1; + next_opaque + } + /// Create a new `Value` for which we have no information at all, except that it is distinct /// from all the others. #[instrument(level = "trace", skip(self), ret)] - fn new_opaque(&mut self) -> Option { - let next_opaque = self.next_opaque.as_mut()?; - let value = Value::Opaque(*next_opaque); - *next_opaque += 1; - Some(self.insert(value)) + fn new_opaque(&mut self) -> VnIndex { + let value = Value::Opaque(self.next_opaque()); + self.insert(value) } /// Create a new `Value::Address` distinct from all the others. #[instrument(level = "trace", skip(self), ret)] - fn new_pointer(&mut self, place: Place<'tcx>, kind: AddressKind) -> Option { - let next_opaque = self.next_opaque.as_mut()?; - let value = Value::Address { place, kind, provenance: *next_opaque }; - *next_opaque += 1; - Some(self.insert(value)) + fn new_pointer(&mut self, place: Place<'tcx>, kind: AddressKind) -> VnIndex { + let value = Value::Address { place, kind, provenance: self.next_opaque() }; + self.insert(value) } fn get(&self, index: VnIndex) -> &Value<'tcx> { @@ -331,21 +329,19 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { } } - fn insert_constant(&mut self, value: Const<'tcx>) -> Option { + fn insert_constant(&mut self, value: Const<'tcx>) -> VnIndex { let disambiguator = if value.is_deterministic() { // The constant is deterministic, no need to disambiguate. 0 } else { // Multiple mentions of this constant will yield different values, // so assign a different `disambiguator` to ensure they do not get the same `VnIndex`. - let next_opaque = self.next_opaque.as_mut()?; - let disambiguator = *next_opaque; - *next_opaque += 1; + let disambiguator = self.next_opaque(); // `disambiguator: 0` means deterministic. debug_assert_ne!(disambiguator, 0); disambiguator }; - Some(self.insert(Value::Constant { value, disambiguator })) + self.insert(Value::Constant { value, disambiguator }) } fn insert_bool(&mut self, flag: bool) -> VnIndex { @@ -797,7 +793,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { location: Location, ) -> Option { match *operand { - Operand::Constant(ref constant) => self.insert_constant(constant.const_), + Operand::Constant(ref constant) => Some(self.insert_constant(constant.const_)), Operand::Copy(ref mut place) | Operand::Move(ref mut place) => { let value = self.simplify_place_value(place, location)?; if let Some(const_) = self.try_as_constant(value) { @@ -833,11 +829,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { Rvalue::Aggregate(..) => return self.simplify_aggregate(rvalue, location), Rvalue::Ref(_, borrow_kind, ref mut place) => { self.simplify_place_projection(place, location); - return self.new_pointer(*place, AddressKind::Ref(borrow_kind)); + return Some(self.new_pointer(*place, AddressKind::Ref(borrow_kind))); } Rvalue::RawPtr(mutbl, ref mut place) => { self.simplify_place_projection(place, location); - return self.new_pointer(*place, AddressKind::Address(mutbl)); + return Some(self.new_pointer(*place, AddressKind::Address(mutbl))); } // Operations. @@ -991,7 +987,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { if is_zst { let ty = rvalue.ty(self.local_decls, tcx); - return self.insert_constant(Const::zero_sized(ty)); + return Some(self.insert_constant(Const::zero_sized(ty))); } } @@ -1020,11 +1016,10 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { } }; - let fields: Option> = field_ops + let mut fields: Vec<_> = field_ops .iter_mut() - .map(|op| self.simplify_operand(op, location).or_else(|| self.new_opaque())) + .map(|op| self.simplify_operand(op, location).unwrap_or_else(|| self.new_opaque())) .collect(); - let mut fields = fields?; if let AggregateTy::RawPtr { data_pointer_ty, output_pointer_ty } = &mut ty { let mut was_updated = false; @@ -1152,11 +1147,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { ) if let ty::Slice(..) = to.builtin_deref(true).unwrap().kind() && let ty::Array(_, len) = from.builtin_deref(true).unwrap().kind() => { - return self.insert_constant(Const::from_ty_const( + return Some(self.insert_constant(Const::from_ty_const( *len, self.tcx.types.usize, self.tcx, - )); + ))); } _ => Value::UnaryOp(op, arg_index), }; @@ -1351,7 +1346,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { if let CastKind::PointerCoercion(ReifyFnPointer | ClosureFnPointer(_), _) = kind { // Each reification of a generic fn may get a different pointer. // Do not try to merge them. - return self.new_opaque(); + return Some(self.new_opaque()); } let mut was_updated = false; @@ -1415,11 +1410,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { // Trivial case: we are fetching a statically known length. let place_ty = place.ty(self.local_decls, self.tcx).ty; if let ty::Array(_, len) = place_ty.kind() { - return self.insert_constant(Const::from_ty_const( + return Some(self.insert_constant(Const::from_ty_const( *len, self.tcx.types.usize, self.tcx, - )); + ))); } let mut inner = self.simplify_place_value(place, location)?; @@ -1441,11 +1436,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { && let Some(to) = to.builtin_deref(true) && let ty::Slice(..) = to.kind() { - return self.insert_constant(Const::from_ty_const( + return Some(self.insert_constant(Const::from_ty_const( *len, self.tcx.types.usize, self.tcx, - )); + ))); } // Fallback: a symbolic `Len`. @@ -1622,7 +1617,7 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> { // `local` as reusable if we have an exact type match. && self.local_decls[local].ty == rvalue.ty(self.local_decls, self.tcx) { - let value = value.or_else(|| self.new_opaque()).unwrap(); + let value = value.unwrap_or_else(|| self.new_opaque()); self.assign(local, value); Some(value) } else { @@ -1649,7 +1644,7 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> { && let Some(local) = destination.as_local() && self.ssa.is_ssa(local) { - let opaque = self.new_opaque().unwrap(); + let opaque = self.new_opaque(); self.assign(local, opaque); } self.super_terminator(terminator, location); From a7c8edd27fa9be546e42a29342e2d20cc67ac4fd Mon Sep 17 00:00:00 2001 From: DianQK Date: Sat, 2 Nov 2024 21:29:53 +0800 Subject: [PATCH 3/4] Invalidate all dereferences --- compiler/rustc_data_structures/src/fx.rs | 2 + compiler/rustc_mir_transform/src/gvn.rs | 40 ++++++++++++--- .../gvn.dereferences.GVN.panic-unwind.diff | 30 ++++------- ...pression_elimination.GVN.panic-unwind.diff | 50 +++++++------------ 4 files changed, 62 insertions(+), 60 deletions(-) diff --git a/compiler/rustc_data_structures/src/fx.rs b/compiler/rustc_data_structures/src/fx.rs index 80e72250470c0..f0db9623b674e 100644 --- a/compiler/rustc_data_structures/src/fx.rs +++ b/compiler/rustc_data_structures/src/fx.rs @@ -9,6 +9,8 @@ pub type FxIndexSet = indexmap::IndexSet>; pub type IndexEntry<'a, K, V> = indexmap::map::Entry<'a, K, V>; pub type IndexOccupiedEntry<'a, K, V> = indexmap::map::OccupiedEntry<'a, K, V>; +pub use indexmap::set::MutableValues; + #[macro_export] macro_rules! define_id_collections { ($map_name:ident, $set_name:ident, $entry_name:ident, $key:ty) => { diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 618001b1d7594..2e7dbad9ac867 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -93,7 +93,7 @@ use rustc_const_eval::interpret::{ ImmTy, Immediate, InterpCx, MemPlaceMeta, MemoryKind, OpTy, Projectable, Scalar, intern_const_alloc_for_constprop, }; -use rustc_data_structures::fx::FxIndexSet; +use rustc_data_structures::fx::{FxIndexSet, MutableValues}; use rustc_data_structures::graph::dominators::Dominators; use rustc_hir::def::DefKind; use rustc_index::bit_set::BitSet; @@ -235,6 +235,8 @@ struct VnState<'body, 'tcx> { evaluated: IndexVec>>, /// Counter to generate different values. next_opaque: usize, + /// Cache the deref values. + derefs: Vec, /// Cache the value of the `unsized_locals` features, to avoid fetching it repeatedly in a loop. feature_unsized_locals: bool, ssa: &'body SsaLocals, @@ -268,6 +270,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { values: FxIndexSet::with_capacity_and_hasher(num_values, Default::default()), evaluated: IndexVec::with_capacity(num_values), next_opaque: 1, + derefs: Vec::new(), feature_unsized_locals: tcx.features().unsized_locals(), ssa, dominators, @@ -362,6 +365,21 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { self.insert(Value::Aggregate(AggregateTy::Tuple, VariantIdx::ZERO, values)) } + fn insert_deref(&mut self, value: VnIndex) -> VnIndex { + let value = self.insert(Value::Projection(value, ProjectionElem::Deref)); + self.derefs.push(value); + value + } + + fn invalidate_derefs(&mut self) { + let mut derefs = Vec::new(); + std::mem::swap(&mut derefs, &mut self.derefs); + for deref in derefs { + let opaque = self.next_opaque(); + *self.values.get_index_mut2(deref.index()).unwrap() = Value::Opaque(opaque); + } + } + #[instrument(level = "trace", skip(self), ret)] fn eval_to_const(&mut self, value: VnIndex) -> Option> { use Value::*; @@ -620,7 +638,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { { // An immutable borrow `_x` always points to the same value for the // lifetime of the borrow, so we can merge all instances of `*_x`. - ProjectionElem::Deref + return Some(self.insert_deref(value)); } else { return None; } @@ -1621,6 +1639,8 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> { self.assign(local, value); Some(value) } else { + // Non-local assignments maybe invalidate deref. + self.invalidate_derefs(); value }; let Some(value) = value else { return }; @@ -1640,12 +1660,16 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> { } fn visit_terminator(&mut self, terminator: &mut Terminator<'tcx>, location: Location) { - if let Terminator { kind: TerminatorKind::Call { destination, .. }, .. } = terminator - && let Some(local) = destination.as_local() - && self.ssa.is_ssa(local) - { - let opaque = self.new_opaque(); - self.assign(local, opaque); + if let Terminator { kind: TerminatorKind::Call { destination, .. }, .. } = terminator { + if let Some(local) = destination.as_local() + && self.ssa.is_ssa(local) + { + let opaque = self.new_opaque(); + self.assign(local, opaque); + } + // Function calls maybe invalidate nested deref, and non-local assignments maybe invalidate deref. + // Currently, no distinction is made between these two cases. + self.invalidate_derefs(); } self.super_terminator(terminator, location); } diff --git a/tests/mir-opt/gvn.dereferences.GVN.panic-unwind.diff b/tests/mir-opt/gvn.dereferences.GVN.panic-unwind.diff index ca6fda483642c..bbca6bc3c754d 100644 --- a/tests/mir-opt/gvn.dereferences.GVN.panic-unwind.diff +++ b/tests/mir-opt/gvn.dereferences.GVN.panic-unwind.diff @@ -107,23 +107,18 @@ StorageLive(_18); _18 = &(*_1); StorageLive(_19); -- StorageLive(_20); -+ nop; + StorageLive(_20); _20 = copy (*_18); -- _19 = opaque::(move _20) -> [return: bb7, unwind continue]; -+ _19 = opaque::(copy _20) -> [return: bb7, unwind continue]; + _19 = opaque::(move _20) -> [return: bb7, unwind continue]; } bb7: { -- StorageDead(_20); -+ nop; + StorageDead(_20); StorageDead(_19); StorageLive(_21); StorageLive(_22); -- _22 = copy (*_18); -- _21 = opaque::(move _22) -> [return: bb8, unwind continue]; -+ _22 = copy _20; -+ _21 = opaque::(copy _20) -> [return: bb8, unwind continue]; + _22 = copy (*_18); + _21 = opaque::(move _22) -> [return: bb8, unwind continue]; } bb8: { @@ -157,23 +152,18 @@ StorageDead(_28); StorageDead(_27); StorageLive(_29); -- StorageLive(_30); -+ nop; + StorageLive(_30); _30 = copy ((*_3).0: u32); -- _29 = opaque::(move _30) -> [return: bb12, unwind continue]; -+ _29 = opaque::(copy _30) -> [return: bb12, unwind continue]; + _29 = opaque::(move _30) -> [return: bb12, unwind continue]; } bb12: { -- StorageDead(_30); -+ nop; + StorageDead(_30); StorageDead(_29); StorageLive(_31); StorageLive(_32); -- _32 = copy ((*_3).0: u32); -- _31 = opaque::(move _32) -> [return: bb13, unwind continue]; -+ _32 = copy _30; -+ _31 = opaque::(copy _30) -> [return: bb13, unwind continue]; + _32 = copy ((*_3).0: u32); + _31 = opaque::(move _32) -> [return: bb13, unwind continue]; } bb13: { diff --git a/tests/mir-opt/gvn.subexpression_elimination.GVN.panic-unwind.diff b/tests/mir-opt/gvn.subexpression_elimination.GVN.panic-unwind.diff index 3ca5238663c28..3996dab27a343 100644 --- a/tests/mir-opt/gvn.subexpression_elimination.GVN.panic-unwind.diff +++ b/tests/mir-opt/gvn.subexpression_elimination.GVN.panic-unwind.diff @@ -758,39 +758,32 @@ StorageLive(_126); _126 = &_3; StorageLive(_127); -- StorageLive(_128); -- StorageLive(_129); -+ nop; -+ nop; + StorageLive(_128); + StorageLive(_129); _129 = copy (*_126); StorageLive(_130); _130 = copy _1; - _128 = Add(move _129, move _130); -+ _128 = Add(copy _129, copy _1); ++ _128 = Add(move _129, copy _1); StorageDead(_130); -- StorageDead(_129); -- _127 = opaque::(move _128) -> [return: bb35, unwind continue]; -+ nop; -+ _127 = opaque::(copy _128) -> [return: bb35, unwind continue]; + StorageDead(_129); + _127 = opaque::(move _128) -> [return: bb35, unwind continue]; } bb35: { -- StorageDead(_128); -+ nop; + StorageDead(_128); StorageDead(_127); StorageLive(_131); StorageLive(_132); StorageLive(_133); -- _133 = copy (*_126); -+ _133 = copy _129; + _133 = copy (*_126); StorageLive(_134); _134 = copy _1; - _132 = Add(move _133, move _134); -+ _132 = copy _128; ++ _132 = Add(move _133, copy _1); StorageDead(_134); StorageDead(_133); -- _131 = opaque::(move _132) -> [return: bb36, unwind continue]; -+ _131 = opaque::(copy _128) -> [return: bb36, unwind continue]; + _131 = opaque::(move _132) -> [return: bb36, unwind continue]; } bb36: { @@ -906,39 +899,32 @@ StorageLive(_163); _163 = &_3; StorageLive(_164); -- StorageLive(_165); -- StorageLive(_166); -+ nop; -+ nop; + StorageLive(_165); + StorageLive(_166); _166 = copy (*_163); StorageLive(_167); _167 = copy _1; - _165 = Add(move _166, move _167); -+ _165 = Add(copy _166, copy _1); ++ _165 = Add(move _166, copy _1); StorageDead(_167); -- StorageDead(_166); -- _164 = opaque::(move _165) -> [return: bb43, unwind continue]; -+ nop; -+ _164 = opaque::(copy _165) -> [return: bb43, unwind continue]; + StorageDead(_166); + _164 = opaque::(move _165) -> [return: bb43, unwind continue]; } bb43: { -- StorageDead(_165); -+ nop; + StorageDead(_165); StorageDead(_164); StorageLive(_168); StorageLive(_169); StorageLive(_170); -- _170 = copy (*_163); -+ _170 = copy _166; + _170 = copy (*_163); StorageLive(_171); _171 = copy _1; - _169 = Add(move _170, move _171); -+ _169 = copy _165; ++ _169 = Add(move _170, copy _1); StorageDead(_171); StorageDead(_170); -- _168 = opaque::(move _169) -> [return: bb44, unwind continue]; -+ _168 = opaque::(copy _165) -> [return: bb44, unwind continue]; + _168 = opaque::(move _169) -> [return: bb44, unwind continue]; } bb44: { From 64d0f489be1ac149b890b474212b83592cf882ec Mon Sep 17 00:00:00 2001 From: DianQK Date: Sat, 2 Nov 2024 22:58:24 +0800 Subject: [PATCH 4/4] Remove `unsound-mir-opts` for `simplify_aggregate_to_copy` --- compiler/rustc_mir_transform/src/gvn.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 2e7dbad9ac867..77601d9885ed7 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -1077,9 +1077,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { } } - // unsound: https://github.com/rust-lang/rust/issues/132353 - if tcx.sess.opts.unstable_opts.unsound_mir_opts - && let AggregateTy::Def(_, _) = ty + if let AggregateTy::Def(_, _) = ty && let Some(value) = self.simplify_aggregate_to_copy(rvalue, location, &fields, variant_index) {