Skip to content

Commit

Permalink
Remove mono-reachable traversal
Browse files Browse the repository at this point in the history
  • Loading branch information
saethlin committed Nov 3, 2024
1 parent ea9304a commit 80015e7
Show file tree
Hide file tree
Showing 8 changed files with 12 additions and 241 deletions.
9 changes: 0 additions & 9 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,19 +302,10 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
.generic_activity("codegen prelude")
.run(|| crate::abi::codegen_fn_prelude(fx, start_block));

let reachable_blocks = traversal::mono_reachable_as_bitset(fx.mir, fx.tcx, fx.instance);

for (bb, bb_data) in fx.mir.basic_blocks.iter_enumerated() {
let block = fx.get_block(bb);
fx.bcx.switch_to_block(block);

if !reachable_blocks.contains(bb) {
// We want to skip this block, because it's not reachable. But we still create
// the block so terminators in other blocks can reference it.
fx.bcx.ins().trap(TrapCode::user(1 /* unreachable */).unwrap());
continue;
}

if bb_data.is_cleanup {
// Unwinding after panicking is not supported
continue;
Expand Down
10 changes: 0 additions & 10 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1253,16 +1253,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}
}

pub(crate) fn codegen_block_as_unreachable(&mut self, bb: mir::BasicBlock) {
let llbb = match self.try_llbb(bb) {
Some(llbb) => llbb,
None => return,
};
let bx = &mut Bx::build(self.cx, llbb);
debug!("codegen_block_as_unreachable({:?})", bb);
bx.unreachable();
}

fn codegen_terminator(
&mut self,
bx: &mut Bx,
Expand Down
14 changes: 2 additions & 12 deletions compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,8 @@ pub fn lower_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
fx.compute_per_local_var_debug_info(&mut start_bx).unzip();
fx.per_local_var_debug_info = per_local_var_debug_info;

let traversal_order = traversal::mono_reachable_reverse_postorder(mir, cx.tcx(), instance);
let traversal_order: Vec<_> =
traversal::reverse_postorder(mir).map(|(block, _data)| block).collect();
let memory_locals = analyze::non_ssa_locals(&fx, &traversal_order);

// Allocate variable and temp allocas
Expand Down Expand Up @@ -282,20 +283,9 @@ pub fn lower_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
// So drop the builder of `start_llbb` to avoid having two at the same time.
drop(start_bx);

let mut unreached_blocks = BitSet::new_filled(mir.basic_blocks.len());
// Codegen the body of each reachable block using our reverse postorder list.
for bb in traversal_order {
fx.codegen_block(bb);
unreached_blocks.remove(bb);
}

// FIXME: These empty unreachable blocks are *mostly* a waste. They are occasionally
// targets for a SwitchInt terminator, but the reimplementation of the mono-reachable
// simplification in SwitchInt lowering sometimes misses cases that
// mono_reachable_reverse_postorder manages to figure out.
// The solution is to do something like post-mono GVN. But for now we have this hack.
for bb in unreached_blocks.iter() {
fx.codegen_block_as_unreachable(bb);
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/basic_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl<'tcx> BasicBlocks<'tcx> {
#[inline]
pub fn reverse_postorder(&self) -> &[BasicBlock] {
self.cache.reverse_postorder.get_or_init(|| {
let mut rpo: Vec<_> = Postorder::new(&self.basic_blocks, START_BLOCK, ()).collect();
let mut rpo: Vec<_> = Postorder::new(&self.basic_blocks, START_BLOCK).collect();
rpo.reverse();
rpo
})
Expand Down
82 changes: 1 addition & 81 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use crate::ty::fold::{FallibleTypeFolder, TypeFoldable};
use crate::ty::print::{FmtPrinter, Printer, pretty_print_const, with_no_trimmed_paths};
use crate::ty::visit::TypeVisitableExt;
use crate::ty::{
self, AdtDef, GenericArg, GenericArgsRef, Instance, InstanceKind, List, Ty, TyCtxt, TypingMode,
self, AdtDef, GenericArg, GenericArgsRef, InstanceKind, List, Ty, TyCtxt, TypingMode,
UserTypeAnnotationIndex,
};

Expand Down Expand Up @@ -617,73 +617,6 @@ impl<'tcx> Body<'tcx> {
self.injection_phase.is_some()
}

/// If this basic block ends with a [`TerminatorKind::SwitchInt`] for which we can evaluate the
/// dimscriminant in monomorphization, we return the discriminant bits and the
/// [`SwitchTargets`], just so the caller doesn't also have to match on the terminator.
fn try_const_mono_switchint<'a>(
tcx: TyCtxt<'tcx>,
instance: Instance<'tcx>,
block: &'a BasicBlockData<'tcx>,
) -> Option<(u128, &'a SwitchTargets)> {
// There are two places here we need to evaluate a constant.
let eval_mono_const = |constant: &ConstOperand<'tcx>| {
let env = ty::ParamEnv::reveal_all();
let mono_literal = instance.instantiate_mir_and_normalize_erasing_regions(
tcx,
env,
crate::ty::EarlyBinder::bind(constant.const_),
);
mono_literal.try_eval_bits(tcx, env)
};

let TerminatorKind::SwitchInt { discr, targets } = &block.terminator().kind else {
return None;
};

// If this is a SwitchInt(const _), then we can just evaluate the constant and return.
let discr = match discr {
Operand::Constant(constant) => {
let bits = eval_mono_const(constant)?;
return Some((bits, targets));
}
Operand::Move(place) | Operand::Copy(place) => place,
};

// MIR for `if false` actually looks like this:
// _1 = const _
// SwitchInt(_1)
//
// And MIR for if intrinsics::ub_checks() looks like this:
// _1 = UbChecks()
// SwitchInt(_1)
//
// So we're going to try to recognize this pattern.
//
// If we have a SwitchInt on a non-const place, we find the most recent statement that
// isn't a storage marker. If that statement is an assignment of a const to our
// discriminant place, we evaluate and return the const, as if we've const-propagated it
// into the SwitchInt.

let last_stmt = block.statements.iter().rev().find(|stmt| {
!matches!(stmt.kind, StatementKind::StorageDead(_) | StatementKind::StorageLive(_))
})?;

let (place, rvalue) = last_stmt.kind.as_assign()?;

if discr != place {
return None;
}

match rvalue {
Rvalue::NullaryOp(NullOp::UbChecks, _) => Some((tcx.sess.ub_checks() as u128, targets)),
Rvalue::Use(Operand::Constant(constant)) => {
let bits = eval_mono_const(constant)?;
Some((bits, targets))
}
_ => None,
}
}

/// For a `Location` in this scope, determine what the "caller location" at that point is. This
/// is interesting because of inlining: the `#[track_caller]` attribute of inlined functions
/// must be honored. Falls back to the `tracked_caller` value for `#[track_caller]` functions,
Expand Down Expand Up @@ -1433,19 +1366,6 @@ impl<'tcx> BasicBlockData<'tcx> {
pub fn is_empty_unreachable(&self) -> bool {
self.statements.is_empty() && matches!(self.terminator().kind, TerminatorKind::Unreachable)
}

/// Like [`Terminator::successors`] but tries to use information available from the [`Instance`]
/// to skip successors like the `false` side of an `if const {`.
///
/// This is used to implement [`traversal::mono_reachable`] and
/// [`traversal::mono_reachable_reverse_postorder`].
pub fn mono_successors(&self, tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> Successors<'_> {
if let Some((bits, targets)) = Body::try_const_mono_switchint(tcx, instance, self) {
targets.successors_for_value(bits)
} else {
self.terminator().successors()
}
}
}

///////////////////////////////////////////////////////////////////////////
Expand Down
130 changes: 5 additions & 125 deletions compiler/rustc_middle/src/mir/traversal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,29 +104,23 @@ impl<'a, 'tcx> Iterator for Preorder<'a, 'tcx> {
/// ```
///
/// A Postorder traversal of this graph is `D B C A` or `D C B A`
pub struct Postorder<'a, 'tcx, C> {
pub struct Postorder<'a, 'tcx> {
basic_blocks: &'a IndexSlice<BasicBlock, BasicBlockData<'tcx>>,
visited: BitSet<BasicBlock>,
visit_stack: Vec<(BasicBlock, Successors<'a>)>,
root_is_start_block: bool,
extra: C,
}

impl<'a, 'tcx, C> Postorder<'a, 'tcx, C>
where
C: Customization<'tcx>,
{
impl<'a, 'tcx> Postorder<'a, 'tcx> {
pub fn new(
basic_blocks: &'a IndexSlice<BasicBlock, BasicBlockData<'tcx>>,
root: BasicBlock,
extra: C,
) -> Postorder<'a, 'tcx, C> {
) -> Postorder<'a, 'tcx> {
let mut po = Postorder {
basic_blocks,
visited: BitSet::new_empty(basic_blocks.len()),
visit_stack: Vec::new(),
root_is_start_block: root == START_BLOCK,
extra,
};

po.visit(root);
Expand All @@ -140,7 +134,7 @@ where
return;
}
let data = &self.basic_blocks[bb];
let successors = C::successors(data, self.extra);
let successors = data.terminator().successors();
self.visit_stack.push((bb, successors));
}

Expand Down Expand Up @@ -198,10 +192,7 @@ where
}
}

impl<'tcx, C> Iterator for Postorder<'_, 'tcx, C>
where
C: Customization<'tcx>,
{
impl<'tcx> Iterator for Postorder<'_, 'tcx> {
type Item = BasicBlock;

fn next(&mut self) -> Option<BasicBlock> {
Expand Down Expand Up @@ -252,29 +243,6 @@ impl<'tcx> Customization<'tcx> for () {
}
}

impl<'tcx> Customization<'tcx> for (TyCtxt<'tcx>, Instance<'tcx>) {
fn successors<'a>(
data: &'a BasicBlockData<'tcx>,
(tcx, instance): (TyCtxt<'tcx>, Instance<'tcx>),
) -> Successors<'a> {
data.mono_successors(tcx, instance)
}
}

pub fn mono_reachable_reverse_postorder<'a, 'tcx>(
body: &'a Body<'tcx>,
tcx: TyCtxt<'tcx>,
instance: Instance<'tcx>,
) -> Vec<BasicBlock> {
let mut iter = Postorder::new(&body.basic_blocks, START_BLOCK, (tcx, instance));
let mut items = Vec::with_capacity(body.basic_blocks.len());
while let Some(block) = iter.next() {
items.push(block);
}
items.reverse();
items
}

/// Returns an iterator over all basic blocks reachable from the `START_BLOCK` in no particular
/// order.
///
Expand Down Expand Up @@ -322,91 +290,3 @@ pub fn reverse_postorder<'a, 'tcx>(
{
body.basic_blocks.reverse_postorder().iter().map(|&bb| (bb, &body.basic_blocks[bb]))
}

/// Traversal of a [`Body`] that tries to avoid unreachable blocks in a monomorphized [`Instance`].
///
/// This is allowed to have false positives; blocks may be visited even if they are not actually
/// reachable.
///
/// Such a traversal is mostly useful because it lets us skip lowering the `false` side
/// of `if <T as Trait>::CONST`, as well as [`NullOp::UbChecks`].
///
/// [`NullOp::UbChecks`]: rustc_middle::mir::NullOp::UbChecks
pub fn mono_reachable<'a, 'tcx>(
body: &'a Body<'tcx>,
tcx: TyCtxt<'tcx>,
instance: Instance<'tcx>,
) -> MonoReachable<'a, 'tcx> {
MonoReachable::new(body, tcx, instance)
}

/// [`MonoReachable`] internally accumulates a [`BitSet`] of visited blocks. This is just a
/// convenience function to run that traversal then extract its set of reached blocks.
pub fn mono_reachable_as_bitset<'a, 'tcx>(
body: &'a Body<'tcx>,
tcx: TyCtxt<'tcx>,
instance: Instance<'tcx>,
) -> BitSet<BasicBlock> {
let mut iter = mono_reachable(body, tcx, instance);
while let Some(_) = iter.next() {}
iter.visited
}

pub struct MonoReachable<'a, 'tcx> {
body: &'a Body<'tcx>,
tcx: TyCtxt<'tcx>,
instance: Instance<'tcx>,
visited: BitSet<BasicBlock>,
// Other traversers track their worklist in a Vec. But we don't care about order, so we can
// store ours in a BitSet and thus save allocations because BitSet has a small size
// optimization.
worklist: BitSet<BasicBlock>,
}

impl<'a, 'tcx> MonoReachable<'a, 'tcx> {
pub fn new(
body: &'a Body<'tcx>,
tcx: TyCtxt<'tcx>,
instance: Instance<'tcx>,
) -> MonoReachable<'a, 'tcx> {
let mut worklist = BitSet::new_empty(body.basic_blocks.len());
worklist.insert(START_BLOCK);
MonoReachable {
body,
tcx,
instance,
visited: BitSet::new_empty(body.basic_blocks.len()),
worklist,
}
}

fn add_work(&mut self, blocks: impl IntoIterator<Item = BasicBlock>) {
for block in blocks.into_iter() {
if !self.visited.contains(block) {
self.worklist.insert(block);
}
}
}
}

impl<'a, 'tcx> Iterator for MonoReachable<'a, 'tcx> {
type Item = (BasicBlock, &'a BasicBlockData<'tcx>);

fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> {
while let Some(idx) = self.worklist.iter().next() {
self.worklist.remove(idx);
if !self.visited.insert(idx) {
continue;
}

let data = &self.body[idx];

let targets = data.mono_successors(self.tcx, self.instance);
self.add_work(targets);

return Some((idx, data));
}

None
}
}
4 changes: 2 additions & 2 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::mir::interpret::{AllocId, ErrorHandled, GlobalAlloc, Scalar};
use rustc_middle::mir::mono::{CollectionMode, InstantiationMode, MonoItem};
use rustc_middle::mir::visit::Visitor as MirVisitor;
use rustc_middle::mir::{self, Location, MentionedItem, traversal};
use rustc_middle::mir::{self, Location, MentionedItem};
use rustc_middle::query::TyCtxtAt;
use rustc_middle::ty::adjustment::{CustomCoerceUnsized, PointerCoercion};
use rustc_middle::ty::layout::ValidityRequirement;
Expand Down Expand Up @@ -1208,7 +1208,7 @@ fn collect_items_of_instance<'tcx>(
};

if mode == CollectionMode::UsedItems {
for (bb, data) in traversal::mono_reachable(body, tcx, instance) {
for (bb, data) in body.basic_blocks.iter_enumerated() {
collector.visit_basic_block_data(bb, data)
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_utils/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub fn visit_local_usage(locals: &[Local], mir: &Body<'_>, location: Location) -
locals.len()
];

traversal::Postorder::new(&mir.basic_blocks, location.block, ())
traversal::Postorder::new(&mir.basic_blocks, location.block)
.collect::<Vec<_>>()
.into_iter()
.rev()
Expand Down

0 comments on commit 80015e7

Please sign in to comment.