From 523f8f8398f84d13c7d38749de53b701ccfb4f50 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sat, 24 Aug 2024 21:03:38 -0400 Subject: [PATCH] Compute reachable locals as part of non_ssa_locals --- compiler/rustc_codegen_ssa/src/mir/analyze.rs | 29 ++-- compiler/rustc_codegen_ssa/src/mir/mod.rs | 12 +- compiler/rustc_middle/src/mir/basic_blocks.rs | 2 +- compiler/rustc_middle/src/mir/traversal.rs | 124 ++++++------------ tests/codegen/constant-branch.rs | 12 +- tests/codegen/no-alloca-inside-if-false.rs | 12 +- 6 files changed, 79 insertions(+), 112 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index 4e4bffd8b89..25712095e76 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs @@ -16,26 +16,16 @@ pub(crate) fn non_ssa_locals<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( fx: &FunctionCx<'a, 'tcx, Bx>, traversal_order: &[mir::BasicBlock], - reachable_locals: &BitSet, ) -> BitSet { let mir = fx.mir; let dominators = mir.basic_blocks.dominators(); let locals = mir .local_decls - .iter_enumerated() - .map(|(local, decl)| { - if !reachable_locals.contains(local) { - return LocalKind::Unused; - } + .iter() + .map(|decl| { let ty = fx.monomorphize(decl.ty); let layout = fx.cx.spanned_layout_of(ty, decl.source_info.span); - if layout.is_zst() { - LocalKind::ZST - } else if fx.cx.is_backend_immediate(layout) || fx.cx.is_backend_scalar_pair(layout) { - LocalKind::Unused - } else { - LocalKind::Memory - } + if layout.is_zst() { LocalKind::ZST } else { LocalKind::Unused } }) .collect(); @@ -83,11 +73,22 @@ struct LocalAnalyzer<'a, 'b, 'tcx, Bx: BuilderMethods<'b, 'tcx>> { impl<'a, 'b, 'tcx, Bx: BuilderMethods<'b, 'tcx>> LocalAnalyzer<'a, 'b, 'tcx, Bx> { fn define(&mut self, local: mir::Local, location: DefLocation) { + let fx = self.fx; let kind = &mut self.locals[local]; + let decl = &fx.mir.local_decls[local]; match *kind { LocalKind::ZST => {} LocalKind::Memory => {} - LocalKind::Unused => *kind = LocalKind::SSA(location), + LocalKind::Unused => { + let ty = fx.monomorphize(decl.ty); + let layout = fx.cx.spanned_layout_of(ty, decl.source_info.span); + *kind = + if fx.cx.is_backend_immediate(layout) || fx.cx.is_backend_scalar_pair(layout) { + LocalKind::SSA(location) + } else { + LocalKind::Memory + }; + } LocalKind::SSA(_) => *kind = LocalKind::Memory, } } diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 6b36e730267..68fb5cb2206 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -192,9 +192,6 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( }) .collect(); - let (traversal_order, reachable_locals) = - traversal::mono_reachable_reverse_postorder(mir, cx.tcx(), instance); - let mut fx = FunctionCx { instance, mir, @@ -221,7 +218,8 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info(&mut start_bx); - let memory_locals = analyze::non_ssa_locals(&fx, &traversal_order, &reachable_locals); + let traversal_order = traversal::mono_reachable_reverse_postorder(mir, cx.tcx(), instance); + let memory_locals = analyze::non_ssa_locals(&fx, &traversal_order); // Allocate variable and temp allocas let local_values = { @@ -286,6 +284,12 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( 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); } diff --git a/compiler/rustc_middle/src/mir/basic_blocks.rs b/compiler/rustc_middle/src/mir/basic_blocks.rs index 183b898253e..63ce47ef327 100644 --- a/compiler/rustc_middle/src/mir/basic_blocks.rs +++ b/compiler/rustc_middle/src/mir/basic_blocks.rs @@ -71,7 +71,7 @@ pub fn predecessors(&self) -> &Predecessors { #[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 }) diff --git a/compiler/rustc_middle/src/mir/traversal.rs b/compiler/rustc_middle/src/mir/traversal.rs index 1d55a38f30b..b8b74da401c 100644 --- a/compiler/rustc_middle/src/mir/traversal.rs +++ b/compiler/rustc_middle/src/mir/traversal.rs @@ -1,5 +1,4 @@ use super::*; -use crate::mir::visit::Visitor; /// Preorder traversal of a graph. /// @@ -105,36 +104,46 @@ fn size_hint(&self) -> (usize, Option) { /// ``` /// /// A Postorder traversal of this graph is `D B C A` or `D C B A` -pub struct Postorder<'a, 'tcx> { +pub struct Postorder<'a, 'tcx, C> { basic_blocks: &'a IndexSlice>, visited: BitSet, visit_stack: Vec<(BasicBlock, Successors<'a>)>, root_is_start_block: bool, + extra: C, } -impl<'a, 'tcx> Postorder<'a, 'tcx> { +impl<'a, 'tcx, C> Postorder<'a, 'tcx, C> +where + C: Customization<'tcx>, +{ pub fn new( basic_blocks: &'a IndexSlice>, root: BasicBlock, - ) -> Postorder<'a, 'tcx> { + extra: C, + ) -> Postorder<'a, 'tcx, C> { 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, }; - let data = &po.basic_blocks[root]; - - if let Some(ref term) = data.terminator { - po.visited.insert(root); - po.visit_stack.push((root, term.successors())); - po.traverse_successor(); - } + po.visit(root); + po.traverse_successor(); po } + fn visit(&mut self, bb: BasicBlock) { + if !self.visited.insert(bb) { + return; + } + let data = &self.basic_blocks[bb]; + let successors = C::successors(data, self.extra); + self.visit_stack.push((bb, successors)); + } + fn traverse_successor(&mut self) { // This is quite a complex loop due to 1. the borrow checker not liking it much // and 2. what exactly is going on is not clear @@ -184,16 +193,15 @@ fn traverse_successor(&mut self) { // since we've already visited `E`, that child isn't added to the stack. The last // two iterations yield `B` and finally `A` for a final traversal of [E, D, C, B, A] while let Some(bb) = self.visit_stack.last_mut().and_then(|(_, iter)| iter.next_back()) { - if self.visited.insert(bb) { - if let Some(term) = &self.basic_blocks[bb].terminator { - self.visit_stack.push((bb, term.successors())); - } - } + self.visit(bb); } } } -impl<'tcx> Iterator for Postorder<'_, 'tcx> { +impl<'tcx, C> Iterator for Postorder<'_, 'tcx, C> +where + C: Customization<'tcx>, +{ type Item = BasicBlock; fn next(&mut self) -> Option { @@ -233,73 +241,23 @@ pub fn postorder<'a, 'tcx>( reverse_postorder(body).rev() } -struct UsedLocals(BitSet); +/// Lets us plug in some additional logic and data into a Postorder traversal. Or not. +pub trait Customization<'tcx>: Copy { + fn successors<'a>(_: &'a BasicBlockData<'tcx>, _: Self) -> Successors<'a>; +} -impl<'tcx> Visitor<'tcx> for UsedLocals { - fn visit_local( - &mut self, - local: Local, - _ctx: crate::mir::visit::PlaceContext, - _location: Location, - ) { - self.0.insert(local); +impl<'tcx> Customization<'tcx> for () { + fn successors<'a>(data: &'a BasicBlockData<'tcx>, _: ()) -> Successors<'a> { + data.terminator().successors() } } -struct MonoReachablePostorder<'a, 'tcx> { - basic_blocks: &'a IndexSlice>, - visited: BitSet, - visit_stack: Vec<(BasicBlock, Successors<'a>)>, - locals: UsedLocals, - tcx: TyCtxt<'tcx>, - instance: Instance<'tcx>, -} - -impl<'a, 'tcx> MonoReachablePostorder<'a, 'tcx> { - fn new( - body: &'a Body<'tcx>, - tcx: TyCtxt<'tcx>, - instance: Instance<'tcx>, - ) -> MonoReachablePostorder<'a, 'tcx> { - let basic_blocks = &body.basic_blocks; - let mut po = MonoReachablePostorder { - basic_blocks, - visited: BitSet::new_empty(basic_blocks.len()), - visit_stack: Vec::new(), - locals: UsedLocals(BitSet::new_empty(body.local_decls.len())), - tcx, - instance, - }; - - po.visit(START_BLOCK); - po.traverse_successor(); - po - } - - fn visit(&mut self, bb: BasicBlock) { - if !self.visited.insert(bb) { - return; - } - let data = &self.basic_blocks[bb]; - self.locals.visit_basic_block_data(bb, data); - let successors = data.mono_successors(self.tcx, self.instance); - self.visit_stack.push((bb, successors)); - } - - fn traverse_successor(&mut self) { - while let Some(bb) = self.visit_stack.last_mut().and_then(|(_, iter)| iter.next_back()) { - self.visit(bb); - } - } -} - -impl<'tcx> Iterator for MonoReachablePostorder<'_, 'tcx> { - type Item = BasicBlock; - - fn next(&mut self) -> Option { - let (bb, _) = self.visit_stack.pop()?; - self.traverse_successor(); - Some(bb) +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) } } @@ -307,14 +265,14 @@ pub fn mono_reachable_reverse_postorder<'a, 'tcx>( body: &'a Body<'tcx>, tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, -) -> (Vec, BitSet) { - let mut iter = MonoReachablePostorder::new(body, tcx, instance); +) -> Vec { + 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, iter.locals.0) + items } /// Returns an iterator over all basic blocks reachable from the `START_BLOCK` in no particular diff --git a/tests/codegen/constant-branch.rs b/tests/codegen/constant-branch.rs index 8f8438b0b1a..8fc8fb4f57a 100644 --- a/tests/codegen/constant-branch.rs +++ b/tests/codegen/constant-branch.rs @@ -7,18 +7,19 @@ // CHECK-LABEL: @if_bool #[no_mangle] pub fn if_bool() { - // CHECK: br label %{{.+}} + // CHECK-NOT: br i1 + // CHECK-NOT: switch _ = if true { 0 } else { 1 }; - // CHECK: br label %{{.+}} _ = if false { 0 } else { 1 }; } // CHECK-LABEL: @if_constant_int_eq #[no_mangle] pub fn if_constant_int_eq() { + // CHECK-NOT: br i1 + // CHECK-NOT: switch let val = 0; - // CHECK: br label %{{.+}} _ = if val == 0 { 0 } else { 1 }; // CHECK: br label %{{.+}} @@ -28,20 +29,19 @@ pub fn if_constant_int_eq() { // CHECK-LABEL: @if_constant_match #[no_mangle] pub fn if_constant_match() { - // CHECK: br label %{{.+}} + // CHECK-NOT: br i1 + // CHECK-NOT: switch _ = match 1 { 1 => 2, 2 => 3, _ => 4, }; - // CHECK: br label %{{.+}} _ = match 1 { 2 => 3, _ => 4, }; - // CHECK: br label %{{.+}} _ = match -1 { -1 => 1, _ => 0, diff --git a/tests/codegen/no-alloca-inside-if-false.rs b/tests/codegen/no-alloca-inside-if-false.rs index 1bfbf71da4d..a231c7e808a 100644 --- a/tests/codegen/no-alloca-inside-if-false.rs +++ b/tests/codegen/no-alloca-inside-if-false.rs @@ -1,4 +1,7 @@ -//@ compile-flags: -Cno-prepopulate-passes -Copt-level=0 +//@ compile-flags: -Cno-prepopulate-passes -Copt-level=0 -Cpanic=abort +// Check that there's an alloca for the reference and the vector, but nothing else. +// We use panic=abort because unwinding panics give hint::black_box a cleanup block, which has +// another alloca. #![crate_type = "lib"] @@ -6,9 +9,8 @@ fn test() { // CHECK-LABEL: no_alloca_inside_if_false::test // CHECK: start: - // CHECK-NEXT: %0 = alloca - // CHECK-NEXT: %vec = alloca - // CHECK-NOT: %arr = alloca + // CHECK-NEXT: alloca [{{12|24}} x i8] + // CHECK-NOT: alloca if const { SIZE < 4096 } { let arr = [0u8; SIZE]; std::hint::black_box(&arr); @@ -18,6 +20,8 @@ fn test() { } } +// CHECK-LABEL: @main +#[no_mangle] pub fn main() { test::<8192>(); }