Skip to content

Commit

Permalink
Remove references from some structs.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nnethercote committed Sep 9, 2024
1 parent d1c55a3 commit 7023402
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 64 deletions.
14 changes: 7 additions & 7 deletions compiler/rustc_mir_transform/src/dest_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -281,20 +281,20 @@ struct Candidates {
fn apply_merges<'tcx>(
body: &mut Body<'tcx>,
tcx: TyCtxt<'tcx>,
merges: &FxIndexMap<Local, Local>,
merged_locals: &BitSet<Local>,
merges: FxIndexMap<Local, Local>,
merged_locals: BitSet<Local>,
) {
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<Local, Local>,
merged_locals: &'a BitSet<Local>,
merges: FxIndexMap<Local, Local>,
merged_locals: BitSet<Local>,
}

impl<'a, 'tcx> MutVisitor<'tcx> for Merger<'a, 'tcx> {
impl<'tcx> MutVisitor<'tcx> for Merger<'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_mir_transform/src/gvn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down Expand Up @@ -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<BasicBlock>,
dominators: Dominators<BasicBlock>,
reused_locals: BitSet<Local>,
}

Expand All @@ -268,7 +268,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
body: &Body<'tcx>,
param_env: ty::ParamEnv<'tcx>,
ssa: &'body SsaLocals,
dominators: &'body Dominators<BasicBlock>,
dominators: Dominators<BasicBlock>,
local_decls: &'body LocalDecls<'tcx>,
) -> Self {
// Compute a rough estimate of the number of values in the body from the number of
Expand Down Expand Up @@ -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()
}
}
Expand Down
42 changes: 20 additions & 22 deletions compiler/rustc_mir_transform/src/jump_threading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};

Expand All @@ -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);
}
Expand All @@ -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<BasicBlock>,
map: Map<'tcx>,
loop_headers: BitSet<BasicBlock>,
/// We use an arena to avoid cloning the slices when cloning `state`.
arena: &'a DroplessArena,
opportunities: Vec<ThreadingOpportunity>,
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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) => {
Expand Down Expand Up @@ -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`.
Expand All @@ -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,
Expand All @@ -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);
}

_ => {}
Expand Down Expand Up @@ -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)) => {
Expand Down Expand Up @@ -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);
}
Expand All @@ -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 };
Expand Down
14 changes: 8 additions & 6 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,24 +223,26 @@ fn is_mir_available(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
/// MIR associated with them.
fn mir_keys(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet<LocalDefId> {
// 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<LocalDefId>,
struct GatherCtors {
set: FxIndexSet<LocalDefId>,
}
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);
}
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 {
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_mir_transform/src/mentioned_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Spanned<MentionedItem<'tcx>>>,
mentioned_items: Vec<Spanned<MentionedItem<'tcx>>>,
}

impl<'tcx> crate::MirPass<'tcx> for MentionedItems {
Expand All @@ -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);
}
}

Expand Down
15 changes: 6 additions & 9 deletions compiler/rustc_mir_transform/src/ref_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<Local, Value<'tcx>>,
struct ReplacementFinder<'tcx, F> {
targets: IndexVec<Local, Value<'tcx>>,
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,
{
Expand Down
19 changes: 7 additions & 12 deletions compiler/rustc_mir_transform/src/required_consts.rs
Original file line number Diff line number Diff line change
@@ -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<ConstOperand<'tcx>>,
pub(super) struct RequiredConstsVisitor<'tcx> {
required_consts: Vec<ConstOperand<'tcx>>,
}

impl<'a, 'tcx> RequiredConstsVisitor<'a, 'tcx> {
fn new(required_consts: &'a mut Vec<ConstOperand<'tcx>>) -> 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);
Expand Down

0 comments on commit 7023402

Please sign in to comment.