From 702340269136b9818359a82c5f7fc659ec26a0bf Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 4 Sep 2024 13:47:26 +1000 Subject: [PATCH] Remove references from some structs. In all cases the struct can own the relevant thing instead of having a reference to it. This makes the code simpler, and in some cases removes a struct lifetime. --- compiler/rustc_mir_transform/src/dest_prop.rs | 14 +++---- compiler/rustc_mir_transform/src/gvn.rs | 8 ++-- .../rustc_mir_transform/src/jump_threading.rs | 42 +++++++++---------- compiler/rustc_mir_transform/src/lib.rs | 14 ++++--- .../src/mentioned_items.rs | 8 ++-- compiler/rustc_mir_transform/src/ref_prop.rs | 15 +++---- .../src/required_consts.rs | 19 ++++----- 7 files changed, 56 insertions(+), 64 deletions(-) diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index 175097e30ee30..f123658580da4 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -242,7 +242,7 @@ impl<'tcx> crate::MirPass<'tcx> for DestinationPropagation { } round_count += 1; - apply_merges(body, tcx, &merges, &merged_locals); + apply_merges(body, tcx, merges, merged_locals); } trace!(round_count); @@ -281,20 +281,20 @@ struct Candidates { fn apply_merges<'tcx>( body: &mut Body<'tcx>, tcx: TyCtxt<'tcx>, - merges: &FxIndexMap, - merged_locals: &BitSet, + merges: FxIndexMap, + merged_locals: BitSet, ) { let mut merger = Merger { tcx, merges, merged_locals }; merger.visit_body_preserves_cfg(body); } -struct Merger<'a, 'tcx> { +struct Merger<'tcx> { tcx: TyCtxt<'tcx>, - merges: &'a FxIndexMap, - merged_locals: &'a BitSet, + merges: FxIndexMap, + merged_locals: BitSet, } -impl<'a, 'tcx> MutVisitor<'tcx> for Merger<'a, 'tcx> { +impl<'tcx> MutVisitor<'tcx> for Merger<'tcx> { fn tcx(&self) -> TyCtxt<'tcx> { self.tcx } diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 0b78affc31cc1..598bf61483d2f 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -125,7 +125,7 @@ impl<'tcx> crate::MirPass<'tcx> for GVN { // Clone dominators because we need them while mutating the body. let dominators = body.basic_blocks.dominators().clone(); - let mut state = VnState::new(tcx, body, param_env, &ssa, &dominators, &body.local_decls); + 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| { @@ -258,7 +258,7 @@ struct VnState<'body, 'tcx> { /// Cache the value of the `unsized_locals` features, to avoid fetching it repeatedly in a loop. feature_unsized_locals: bool, ssa: &'body SsaLocals, - dominators: &'body Dominators, + dominators: Dominators, reused_locals: BitSet, } @@ -268,7 +268,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { body: &Body<'tcx>, param_env: ty::ParamEnv<'tcx>, ssa: &'body SsaLocals, - dominators: &'body Dominators, + dominators: Dominators, local_decls: &'body LocalDecls<'tcx>, ) -> Self { // Compute a rough estimate of the number of values in the body from the number of @@ -1490,7 +1490,7 @@ impl<'tcx> VnState<'_, 'tcx> { let other = self.rev_locals.get(index)?; other .iter() - .find(|&&other| self.ssa.assignment_dominates(self.dominators, other, loc)) + .find(|&&other| self.ssa.assignment_dominates(&self.dominators, other, loc)) .copied() } } diff --git a/compiler/rustc_mir_transform/src/jump_threading.rs b/compiler/rustc_mir_transform/src/jump_threading.rs index e950c2cb72ab8..d48df59ada860 100644 --- a/compiler/rustc_mir_transform/src/jump_threading.rs +++ b/compiler/rustc_mir_transform/src/jump_threading.rs @@ -78,18 +78,16 @@ impl<'tcx> crate::MirPass<'tcx> for JumpThreading { } let param_env = tcx.param_env_reveal_all_normalized(def_id); - let map = Map::new(tcx, body, Some(MAX_PLACES)); - let loop_headers = loop_headers(body); - let arena = DroplessArena::default(); + let arena = &DroplessArena::default(); let mut finder = TOFinder { tcx, param_env, ecx: InterpCx::new(tcx, DUMMY_SP, param_env, DummyMachine), body, - arena: &arena, - map: &map, - loop_headers: &loop_headers, + arena, + map: Map::new(tcx, body, Some(MAX_PLACES)), + loop_headers: loop_headers(body), opportunities: Vec::new(), }; @@ -105,7 +103,7 @@ impl<'tcx> crate::MirPass<'tcx> for JumpThreading { // Verify that we do not thread through a loop header. for to in opportunities.iter() { - assert!(to.chain.iter().all(|&block| !loop_headers.contains(block))); + assert!(to.chain.iter().all(|&block| !finder.loop_headers.contains(block))); } OpportunitySet::new(body, opportunities).apply(body); } @@ -124,8 +122,8 @@ struct TOFinder<'tcx, 'a> { param_env: ty::ParamEnv<'tcx>, ecx: InterpCx<'tcx, DummyMachine>, body: &'a Body<'tcx>, - map: &'a Map<'tcx>, - loop_headers: &'a BitSet, + map: Map<'tcx>, + loop_headers: BitSet, /// We use an arena to avoid cloning the slices when cloning `state`. arena: &'a DroplessArena, opportunities: Vec, @@ -223,7 +221,7 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { })) }; let conds = ConditionSet(conds); - state.insert_value_idx(discr, conds, self.map); + state.insert_value_idx(discr, conds, &self.map); self.find_opportunity(bb, state, cost, 0); } @@ -264,7 +262,7 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { // _1 = 5 // Whatever happens here, it won't change the result of a `SwitchInt`. // _1 = 6 if let Some((lhs, tail)) = self.mutated_statement(stmt) { - state.flood_with_tail_elem(lhs.as_ref(), tail, self.map, ConditionSet::BOTTOM); + state.flood_with_tail_elem(lhs.as_ref(), tail, &self.map, ConditionSet::BOTTOM); } } @@ -370,7 +368,7 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { self.opportunities.push(ThreadingOpportunity { chain: vec![bb], target: c.target }) }; - if let Some(conditions) = state.try_get_idx(lhs, self.map) + if let Some(conditions) = state.try_get_idx(lhs, &self.map) && let Immediate::Scalar(Scalar::Int(int)) = *rhs { conditions.iter_matches(int).for_each(register_opportunity); @@ -406,7 +404,7 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { } }, &mut |place, op| { - if let Some(conditions) = state.try_get_idx(place, self.map) + if let Some(conditions) = state.try_get_idx(place, &self.map) && let Ok(imm) = self.ecx.read_immediate_raw(op) && let Some(imm) = imm.right() && let Immediate::Scalar(Scalar::Int(int)) = *imm @@ -441,7 +439,7 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { // Transfer the conditions on the copied rhs. Operand::Move(rhs) | Operand::Copy(rhs) => { let Some(rhs) = self.map.find(rhs.as_ref()) else { return }; - state.insert_place_idx(rhs, lhs, self.map); + state.insert_place_idx(rhs, lhs, &self.map); } } } @@ -461,7 +459,7 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { Rvalue::CopyForDeref(rhs) => self.process_operand(bb, lhs, &Operand::Copy(*rhs), state), Rvalue::Discriminant(rhs) => { let Some(rhs) = self.map.find_discr(rhs.as_ref()) else { return }; - state.insert_place_idx(rhs, lhs, self.map); + state.insert_place_idx(rhs, lhs, &self.map); } // If we expect `lhs ?= A`, we have an opportunity if we assume `constant == A`. Rvalue::Aggregate(box ref kind, ref operands) => { @@ -492,10 +490,10 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { } // Transfer the conditions on the copy rhs, after inversing polarity. Rvalue::UnaryOp(UnOp::Not, Operand::Move(place) | Operand::Copy(place)) => { - let Some(conditions) = state.try_get_idx(lhs, self.map) else { return }; + let Some(conditions) = state.try_get_idx(lhs, &self.map) else { return }; let Some(place) = self.map.find(place.as_ref()) else { return }; let conds = conditions.map(self.arena, Condition::inv); - state.insert_value_idx(place, conds, self.map); + state.insert_value_idx(place, conds, &self.map); } // We expect `lhs ?= A`. We found `lhs = Eq(rhs, B)`. // Create a condition on `rhs ?= B`. @@ -504,7 +502,7 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { box (Operand::Move(place) | Operand::Copy(place), Operand::Constant(value)) | box (Operand::Constant(value), Operand::Move(place) | Operand::Copy(place)), ) => { - let Some(conditions) = state.try_get_idx(lhs, self.map) else { return }; + let Some(conditions) = state.try_get_idx(lhs, &self.map) else { return }; let Some(place) = self.map.find(place.as_ref()) else { return }; let equals = match op { BinOp::Eq => ScalarInt::TRUE, @@ -528,7 +526,7 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { polarity: if c.matches(equals) { Polarity::Eq } else { Polarity::Ne }, ..c }); - state.insert_value_idx(place, conds, self.map); + state.insert_value_idx(place, conds, &self.map); } _ => {} @@ -583,7 +581,7 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume( Operand::Copy(place) | Operand::Move(place), )) => { - let Some(conditions) = state.try_get(place.as_ref(), self.map) else { return }; + let Some(conditions) = state.try_get(place.as_ref(), &self.map) else { return }; conditions.iter_matches(ScalarInt::TRUE).for_each(register_opportunity); } StatementKind::Assign(box (lhs_place, rhs)) => { @@ -631,7 +629,7 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { // We can recurse through this terminator. let mut state = state(); if let Some(place_to_flood) = place_to_flood { - state.flood_with(place_to_flood.as_ref(), self.map, ConditionSet::BOTTOM); + state.flood_with(place_to_flood.as_ref(), &self.map, ConditionSet::BOTTOM); } self.find_opportunity(bb, state, cost.clone(), depth + 1); } @@ -650,7 +648,7 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { let Some(discr) = discr.place() else { return }; let discr_ty = discr.ty(self.body, self.tcx).ty; let Ok(discr_layout) = self.ecx.layout_of(discr_ty) else { return }; - let Some(conditions) = state.try_get(discr.as_ref(), self.map) else { return }; + let Some(conditions) = state.try_get(discr.as_ref(), &self.map) else { return }; if let Some((value, _)) = targets.iter().find(|&(_, target)| target == target_bb) { let Some(value) = ScalarInt::try_from_uint(value, discr_layout.size) else { return }; diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index dcfcad7cc7c5b..6c9b46d8b6f6a 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -223,14 +223,14 @@ fn is_mir_available(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { /// MIR associated with them. fn mir_keys(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet { // All body-owners have MIR associated with them. - let mut set: FxIndexSet<_> = tcx.hir().body_owners().collect(); + let set: FxIndexSet<_> = tcx.hir().body_owners().collect(); // Additionally, tuple struct/variant constructors have MIR, but // they don't have a BodyId, so we need to build them separately. - struct GatherCtors<'a> { - set: &'a mut FxIndexSet, + struct GatherCtors { + set: FxIndexSet, } - impl<'tcx> Visitor<'tcx> for GatherCtors<'_> { + impl<'tcx> Visitor<'tcx> for GatherCtors { fn visit_variant_data(&mut self, v: &'tcx hir::VariantData<'tcx>) { if let hir::VariantData::Tuple(_, _, def_id) = *v { self.set.insert(def_id); @@ -238,9 +238,11 @@ fn mir_keys(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet { intravisit::walk_struct_def(self, v) } } - tcx.hir().visit_all_item_likes_in_crate(&mut GatherCtors { set: &mut set }); - set + let mut gather_ctors = GatherCtors { set }; + tcx.hir().visit_all_item_likes_in_crate(&mut gather_ctors); + + gather_ctors.set } fn mir_const_qualif(tcx: TyCtxt<'_>, def: LocalDefId) -> ConstQualifs { diff --git a/compiler/rustc_mir_transform/src/mentioned_items.rs b/compiler/rustc_mir_transform/src/mentioned_items.rs index 4402e0dd42d40..f24de609e6b29 100644 --- a/compiler/rustc_mir_transform/src/mentioned_items.rs +++ b/compiler/rustc_mir_transform/src/mentioned_items.rs @@ -10,7 +10,7 @@ pub(super) struct MentionedItems; struct MentionedItemsVisitor<'a, 'tcx> { tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, - mentioned_items: &'a mut Vec>>, + mentioned_items: Vec>>, } impl<'tcx> crate::MirPass<'tcx> for MentionedItems { @@ -23,9 +23,9 @@ impl<'tcx> crate::MirPass<'tcx> for MentionedItems { } fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut mir::Body<'tcx>) { - let mut mentioned_items = Vec::new(); - MentionedItemsVisitor { tcx, body, mentioned_items: &mut mentioned_items }.visit_body(body); - body.set_mentioned_items(mentioned_items); + let mut visitor = MentionedItemsVisitor { tcx, body, mentioned_items: Vec::new() }; + visitor.visit_body(body); + body.set_mentioned_items(visitor.mentioned_items); } } diff --git a/compiler/rustc_mir_transform/src/ref_prop.rs b/compiler/rustc_mir_transform/src/ref_prop.rs index 25b98786c66b7..4c3a99b79d4f2 100644 --- a/compiler/rustc_mir_transform/src/ref_prop.rs +++ b/compiler/rustc_mir_transform/src/ref_prop.rs @@ -253,11 +253,8 @@ fn compute_replacement<'tcx>( debug!(?targets); - let mut finder = ReplacementFinder { - targets: &mut targets, - can_perform_opt, - allowed_replacements: FxHashSet::default(), - }; + let mut finder = + ReplacementFinder { targets, can_perform_opt, allowed_replacements: FxHashSet::default() }; let reachable_blocks = traversal::reachable_as_bitset(body); for (bb, bbdata) in body.basic_blocks.iter_enumerated() { // Only visit reachable blocks as we rely on dataflow. @@ -269,19 +266,19 @@ fn compute_replacement<'tcx>( let allowed_replacements = finder.allowed_replacements; return Replacer { tcx, - targets, + targets: finder.targets, storage_to_remove, allowed_replacements, any_replacement: false, }; - struct ReplacementFinder<'a, 'tcx, F> { - targets: &'a mut IndexVec>, + struct ReplacementFinder<'tcx, F> { + targets: IndexVec>, can_perform_opt: F, allowed_replacements: FxHashSet<(Local, Location)>, } - impl<'tcx, F> Visitor<'tcx> for ReplacementFinder<'_, 'tcx, F> + impl<'tcx, F> Visitor<'tcx> for ReplacementFinder<'tcx, F> where F: FnMut(Place<'tcx>, Location) -> bool, { diff --git a/compiler/rustc_mir_transform/src/required_consts.rs b/compiler/rustc_mir_transform/src/required_consts.rs index ebcf5b5d27b84..99d1cd6f63ecf 100644 --- a/compiler/rustc_mir_transform/src/required_consts.rs +++ b/compiler/rustc_mir_transform/src/required_consts.rs @@ -1,26 +1,21 @@ use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::{traversal, Body, ConstOperand, Location}; -pub(super) struct RequiredConstsVisitor<'a, 'tcx> { - required_consts: &'a mut Vec>, +pub(super) struct RequiredConstsVisitor<'tcx> { + required_consts: Vec>, } -impl<'a, 'tcx> RequiredConstsVisitor<'a, 'tcx> { - fn new(required_consts: &'a mut Vec>) -> Self { - RequiredConstsVisitor { required_consts } - } - +impl<'tcx> RequiredConstsVisitor<'tcx> { pub(super) fn compute_required_consts(body: &mut Body<'tcx>) { - let mut required_consts = Vec::new(); - let mut required_consts_visitor = RequiredConstsVisitor::new(&mut required_consts); + let mut visitor = RequiredConstsVisitor { required_consts: Vec::new() }; for (bb, bb_data) in traversal::reverse_postorder(&body) { - required_consts_visitor.visit_basic_block_data(bb, bb_data); + visitor.visit_basic_block_data(bb, bb_data); } - body.set_required_consts(required_consts); + body.set_required_consts(visitor.required_consts); } } -impl<'tcx> Visitor<'tcx> for RequiredConstsVisitor<'_, 'tcx> { +impl<'tcx> Visitor<'tcx> for RequiredConstsVisitor<'tcx> { fn visit_const_operand(&mut self, constant: &ConstOperand<'tcx>, _: Location) { if constant.const_.is_required_const() { self.required_consts.push(*constant);