Skip to content

Commit bb78dba

Browse files
committed
Auto merge of rust-lang#123272 - saethlin:reachable-mono-cleanup, r=cjgillot
Only collect mono items from reachable blocks Fixes the wrong comment pointed out in: rust-lang#121421 (comment) Moves the analysis to use the worklist strategy: rust-lang#121421 (comment) Also fixes rust-lang#85836, using the same reachability analysis
2 parents 86b603c + b5b4928 commit bb78dba

File tree

5 files changed

+111
-68
lines changed

5 files changed

+111
-68
lines changed

compiler/rustc_codegen_cranelift/src/base.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
267267
.generic_activity("codegen prelude")
268268
.run(|| crate::abi::codegen_fn_prelude(fx, start_block));
269269

270-
for (bb, bb_data) in fx.mir.basic_blocks.iter_enumerated() {
270+
for (bb, bb_data) in traversal::mono_reachable(fx.mir, fx.tcx, fx.instance) {
271271
let block = fx.get_block(bb);
272272
fx.bcx.switch_to_block(block);
273273

compiler/rustc_codegen_ssa/src/mir/mod.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -257,20 +257,19 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
257257
// Apply debuginfo to the newly allocated locals.
258258
fx.debug_introduce_locals(&mut start_bx);
259259

260-
let reachable_blocks = mir.reachable_blocks_in_mono(cx.tcx(), instance);
261-
262260
// The builders will be created separately for each basic block at `codegen_block`.
263261
// So drop the builder of `start_llbb` to avoid having two at the same time.
264262
drop(start_bx);
265263

264+
let reachable_blocks = traversal::mono_reachable_as_bitset(mir, cx.tcx(), instance);
265+
266266
// Codegen the body of each block using reverse postorder
267267
for (bb, _) in traversal::reverse_postorder(mir) {
268268
if reachable_blocks.contains(bb) {
269269
fx.codegen_block(bb);
270270
} else {
271-
// This may have references to things we didn't monomorphize, so we
272-
// don't actually codegen the body. We still create the block so
273-
// terminators in other blocks can reference it without worry.
271+
// We want to skip this block, because it's not reachable. But we still create
272+
// the block so terminators in other blocks can reference it.
274273
fx.codegen_block_as_unreachable(bb);
275274
}
276275
}

compiler/rustc_middle/src/mir/mod.rs

-52
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ pub use rustc_ast::Mutability;
3030
use rustc_data_structures::fx::FxHashMap;
3131
use rustc_data_structures::fx::FxHashSet;
3232
use rustc_data_structures::graph::dominators::Dominators;
33-
use rustc_data_structures::stack::ensure_sufficient_stack;
3433
use rustc_index::bit_set::BitSet;
3534
use rustc_index::{Idx, IndexSlice, IndexVec};
3635
use rustc_serialize::{Decodable, Encodable};
@@ -687,57 +686,6 @@ impl<'tcx> Body<'tcx> {
687686
self.injection_phase.is_some()
688687
}
689688

690-
/// Finds which basic blocks are actually reachable for a specific
691-
/// monomorphization of this body.
692-
///
693-
/// This is allowed to have false positives; just because this says a block
694-
/// is reachable doesn't mean that's necessarily true. It's thus always
695-
/// legal for this to return a filled set.
696-
///
697-
/// Regardless, the [`BitSet::domain_size`] of the returned set will always
698-
/// exactly match the number of blocks in the body so that `contains`
699-
/// checks can be done without worrying about panicking.
700-
///
701-
/// This is mostly useful because it lets us skip lowering the `false` side
702-
/// of `if <T as Trait>::CONST`, as well as `intrinsics::debug_assertions`.
703-
pub fn reachable_blocks_in_mono(
704-
&self,
705-
tcx: TyCtxt<'tcx>,
706-
instance: Instance<'tcx>,
707-
) -> BitSet<BasicBlock> {
708-
let mut set = BitSet::new_empty(self.basic_blocks.len());
709-
self.reachable_blocks_in_mono_from(tcx, instance, &mut set, START_BLOCK);
710-
set
711-
}
712-
713-
fn reachable_blocks_in_mono_from(
714-
&self,
715-
tcx: TyCtxt<'tcx>,
716-
instance: Instance<'tcx>,
717-
set: &mut BitSet<BasicBlock>,
718-
bb: BasicBlock,
719-
) {
720-
if !set.insert(bb) {
721-
return;
722-
}
723-
724-
let data = &self.basic_blocks[bb];
725-
726-
if let Some((bits, targets)) = Self::try_const_mono_switchint(tcx, instance, data) {
727-
let target = targets.target_for_value(bits);
728-
ensure_sufficient_stack(|| {
729-
self.reachable_blocks_in_mono_from(tcx, instance, set, target)
730-
});
731-
return;
732-
}
733-
734-
for target in data.terminator().successors() {
735-
ensure_sufficient_stack(|| {
736-
self.reachable_blocks_in_mono_from(tcx, instance, set, target)
737-
});
738-
}
739-
}
740-
741689
/// If this basic block ends with a [`TerminatorKind::SwitchInt`] for which we can evaluate the
742690
/// dimscriminant in monomorphization, we return the discriminant bits and the
743691
/// [`SwitchTargets`], just so the caller doesn't also have to match on the terminator.

compiler/rustc_middle/src/mir/traversal.rs

+95-1
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ pub fn reachable<'a, 'tcx>(
245245
/// Returns a `BitSet` containing all basic blocks reachable from the `START_BLOCK`.
246246
pub fn reachable_as_bitset(body: &Body<'_>) -> BitSet<BasicBlock> {
247247
let mut iter = preorder(body);
248-
iter.by_ref().for_each(drop);
248+
while let Some(_) = iter.next() {}
249249
iter.visited
250250
}
251251

@@ -279,3 +279,97 @@ pub fn reverse_postorder<'a, 'tcx>(
279279
{
280280
body.basic_blocks.reverse_postorder().iter().map(|&bb| (bb, &body.basic_blocks[bb]))
281281
}
282+
283+
/// Traversal of a [`Body`] that tries to avoid unreachable blocks in a monomorphized [`Instance`].
284+
///
285+
/// This is allowed to have false positives; blocks may be visited even if they are not actually
286+
/// reachable.
287+
///
288+
/// Such a traversal is mostly useful because it lets us skip lowering the `false` side
289+
/// of `if <T as Trait>::CONST`, as well as [`NullOp::UbChecks`].
290+
///
291+
/// [`NullOp::UbChecks`]: rustc_middle::mir::NullOp::UbChecks
292+
pub fn mono_reachable<'a, 'tcx>(
293+
body: &'a Body<'tcx>,
294+
tcx: TyCtxt<'tcx>,
295+
instance: Instance<'tcx>,
296+
) -> MonoReachable<'a, 'tcx> {
297+
MonoReachable::new(body, tcx, instance)
298+
}
299+
300+
/// [`MonoReachable`] internally accumulates a [`BitSet`] of visited blocks. This is just a
301+
/// convenience function to run that traversal then extract its set of reached blocks.
302+
pub fn mono_reachable_as_bitset<'a, 'tcx>(
303+
body: &'a Body<'tcx>,
304+
tcx: TyCtxt<'tcx>,
305+
instance: Instance<'tcx>,
306+
) -> BitSet<BasicBlock> {
307+
let mut iter = mono_reachable(body, tcx, instance);
308+
while let Some(_) = iter.next() {}
309+
iter.visited
310+
}
311+
312+
pub struct MonoReachable<'a, 'tcx> {
313+
body: &'a Body<'tcx>,
314+
tcx: TyCtxt<'tcx>,
315+
instance: Instance<'tcx>,
316+
visited: BitSet<BasicBlock>,
317+
// Other traversers track their worklist in a Vec. But we don't care about order, so we can
318+
// store ours in a BitSet and thus save allocations because BitSet has a small size
319+
// optimization.
320+
worklist: BitSet<BasicBlock>,
321+
}
322+
323+
impl<'a, 'tcx> MonoReachable<'a, 'tcx> {
324+
pub fn new(
325+
body: &'a Body<'tcx>,
326+
tcx: TyCtxt<'tcx>,
327+
instance: Instance<'tcx>,
328+
) -> MonoReachable<'a, 'tcx> {
329+
let mut worklist = BitSet::new_empty(body.basic_blocks.len());
330+
worklist.insert(START_BLOCK);
331+
MonoReachable {
332+
body,
333+
tcx,
334+
instance,
335+
visited: BitSet::new_empty(body.basic_blocks.len()),
336+
worklist,
337+
}
338+
}
339+
340+
fn add_work(&mut self, blocks: impl IntoIterator<Item = BasicBlock>) {
341+
for block in blocks.into_iter() {
342+
if !self.visited.contains(block) {
343+
self.worklist.insert(block);
344+
}
345+
}
346+
}
347+
}
348+
349+
impl<'a, 'tcx> Iterator for MonoReachable<'a, 'tcx> {
350+
type Item = (BasicBlock, &'a BasicBlockData<'tcx>);
351+
352+
fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> {
353+
while let Some(idx) = self.worklist.iter().next() {
354+
self.worklist.remove(idx);
355+
if !self.visited.insert(idx) {
356+
continue;
357+
}
358+
359+
let data = &self.body[idx];
360+
361+
if let Some((bits, targets)) =
362+
Body::try_const_mono_switchint(self.tcx, self.instance, data)
363+
{
364+
let target = targets.target_for_value(bits);
365+
self.add_work([target]);
366+
} else {
367+
self.add_work(data.terminator().successors());
368+
}
369+
370+
return Some((idx, data));
371+
}
372+
373+
None
374+
}
375+
}

compiler/rustc_monomorphize/src/collector.rs

+11-9
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ use rustc_hir::lang_items::LangItem;
214214
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
215215
use rustc_middle::mir::interpret::{AllocId, ErrorHandled, GlobalAlloc, Scalar};
216216
use rustc_middle::mir::mono::{InstantiationMode, MonoItem};
217+
use rustc_middle::mir::traversal;
217218
use rustc_middle::mir::visit::Visitor as MirVisitor;
218219
use rustc_middle::mir::{self, Location, MentionedItem};
219220
use rustc_middle::query::TyCtxtAt;
@@ -1414,15 +1415,16 @@ fn collect_items_of_instance<'tcx>(
14141415
};
14151416

14161417
if mode == CollectionMode::UsedItems {
1417-
// Visit everything. Here we rely on the visitor also visiting `required_consts`, so that we
1418-
// evaluate them and abort compilation if any of them errors.
1419-
collector.visit_body(body);
1420-
} else {
1421-
// We only need to evaluate all constants, but can ignore the rest of the MIR.
1422-
for const_op in &body.required_consts {
1423-
if let Some(val) = collector.eval_constant(const_op) {
1424-
collect_const_value(tcx, val, mentioned_items);
1425-
}
1418+
for (bb, data) in traversal::mono_reachable(body, tcx, instance) {
1419+
collector.visit_basic_block_data(bb, data)
1420+
}
1421+
}
1422+
1423+
// Always visit all `required_consts`, so that we evaluate them and abort compilation if any of
1424+
// them errors.
1425+
for const_op in &body.required_consts {
1426+
if let Some(val) = collector.eval_constant(const_op) {
1427+
collect_const_value(tcx, val, mentioned_items);
14261428
}
14271429
}
14281430

0 commit comments

Comments
 (0)