From b5d6de24a4f982cd34385bc81659c9d3c7cb231a Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Thu, 25 Nov 2021 23:29:21 -0800 Subject: [PATCH] Skip those blocks in codegen too, to avoid the linker errors And hey, maybe even be faster because it means less IR emitted. --- compiler/rustc_codegen_ssa/src/mir/block.rs | 6 ++ compiler/rustc_codegen_ssa/src/mir/mod.rs | 11 ++- compiler/rustc_middle/src/mir/mod.rs | 71 +++++++++++++- compiler/rustc_middle/src/mir/terminator.rs | 7 ++ .../src/simplify_branches.rs | 11 +-- compiler/rustc_monomorphize/src/collector.rs | 92 +------------------ src/test/codegen/skip-mono-inside-if-false.rs | 12 ++- .../const_prop/control-flow-simplification.rs | 2 +- .../mir-opt/simplify_if_generic_constant.rs | 12 +-- ....use_associated_const.SimplifyIfConst.diff | 20 ++-- .../control-flow/drop-fail.precise.stderr | 2 +- src/test/ui/consts/control-flow/drop-fail.rs | 7 +- .../control-flow/drop-fail.stock.stderr | 6 +- 13 files changed, 132 insertions(+), 127 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index c8f388bfa1d5a..665a0b22b47c3 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -956,6 +956,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { self.codegen_terminator(bx, bb, data.terminator()); } + pub fn codegen_block_as_unreachable(&mut self, bb: mir::BasicBlock) { + let mut bx = self.build_block(bb); + debug!("codegen_block_as_unreachable({:?})", bb); + bx.unreachable(); + } + fn codegen_terminator( &mut self, mut bx: Bx, diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 1ef863e84af7f..60b377bbc9cc6 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -240,10 +240,19 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( // Apply debuginfo to the newly allocated locals. fx.debug_introduce_locals(&mut bx); + let reachable_blocks = mir.reachable_blocks_in_mono(cx.tcx(), instance); + // Codegen the body of each block using reverse postorder // FIXME(eddyb) reuse RPO iterator between `analysis` and this. for (bb, _) in traversal::reverse_postorder(&mir) { - fx.codegen_block(bb); + if reachable_blocks.contains(bb) { + fx.codegen_block(bb); + } else { + // This may have references to things we didn't monomorphize, so we + // don't actually codegen the body. We still create the block so + // terminators in other blocks can reference it without worry. + fx.codegen_block_as_unreachable(bb); + } } // For backends that support CFI using type membership (i.e., testing whether a given pointer diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index d113dc696a1bc..d5814a1c08613 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -11,7 +11,7 @@ use crate::ty::fold::{TypeFoldable, TypeFolder, TypeVisitor}; use crate::ty::print::{FmtPrinter, Printer}; use crate::ty::subst::{Subst, SubstsRef}; use crate::ty::{self, List, Ty, TyCtxt}; -use crate::ty::{AdtDef, InstanceDef, Region, ScalarInt, UserTypeAnnotationIndex}; +use crate::ty::{AdtDef, Instance, InstanceDef, Region, ScalarInt, UserTypeAnnotationIndex}; use rustc_hir::def::{CtorKind, Namespace}; use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX}; use rustc_hir::{self, GeneratorKind}; @@ -23,7 +23,7 @@ pub use rustc_ast::Mutability; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::graph::dominators::{dominators, Dominators}; use rustc_data_structures::graph::{self, GraphSuccessors}; -use rustc_index::bit_set::BitMatrix; +use rustc_index::bit_set::{BitMatrix, BitSet}; use rustc_index::vec::{Idx, IndexVec}; use rustc_serialize::{Decodable, Encodable}; use rustc_span::symbol::Symbol; @@ -517,6 +517,73 @@ impl<'tcx> Body<'tcx> { pub fn generator_kind(&self) -> Option { self.generator.as_ref().map(|generator| generator.generator_kind) } + + /// Finds which basic blocks are actually reachable for a specific + /// monomorphization of this body. + /// + /// This is allowed to have false positives; just because this says a block + /// is reachable doesn't mean that's necessarily true. It's thus always + /// legal for this to return a filled set. + /// + /// Regardless, the [`BitSet::domain_size`] of the returned set will always + /// exactly match the number of blocks in the body so that `contains` + /// checks can be done without worrying about panicking. + /// + /// The main case this supports is filtering out `if ::CONST` + /// bodies that can't be removed in generic MIR, but *can* be removed once + /// the specific `T` is known. + /// + /// This is used in the monomorphization collector as well as in codegen. + pub fn reachable_blocks_in_mono( + &self, + tcx: TyCtxt<'tcx>, + instance: Instance<'tcx>, + ) -> BitSet { + if instance.substs.is_noop() { + // If it's non-generic, then mir-opt const prop has already run, meaning it's + // probably not worth doing any further filtering. So call everything reachable. + return BitSet::new_filled(self.basic_blocks().len()); + } + + let mut set = BitSet::new_empty(self.basic_blocks().len()); + self.reachable_blocks_in_mono_from(tcx, instance, &mut set, START_BLOCK); + set + } + + fn reachable_blocks_in_mono_from( + &self, + tcx: TyCtxt<'tcx>, + instance: Instance<'tcx>, + set: &mut BitSet, + bb: BasicBlock, + ) { + if !set.insert(bb) { + return; + } + + let data = &self.basic_blocks()[bb]; + + if let TerminatorKind::SwitchInt { + discr: Operand::Constant(constant), + switch_ty, + targets, + } = &data.terminator().kind + { + let env = ty::ParamEnv::reveal_all(); + let mono_literal = + instance.subst_mir_and_normalize_erasing_regions(tcx, env, constant.literal); + if let Some(bits) = mono_literal.try_eval_bits(tcx, env, switch_ty) { + let target = targets.target_for_value(bits); + return self.reachable_blocks_in_mono_from(tcx, instance, set, target); + } else { + bug!("Couldn't evaluate constant {:?} in mono {:?}", constant, instance); + } + } + + for &target in data.terminator().successors() { + self.reachable_blocks_in_mono_from(tcx, instance, set, target); + } + } } #[derive(Copy, Clone, PartialEq, Eq, Debug, TyEncodable, TyDecodable, HashStable)] diff --git a/compiler/rustc_middle/src/mir/terminator.rs b/compiler/rustc_middle/src/mir/terminator.rs index e78b6fd092de2..e33fb1acb6e60 100644 --- a/compiler/rustc_middle/src/mir/terminator.rs +++ b/compiler/rustc_middle/src/mir/terminator.rs @@ -78,6 +78,13 @@ impl SwitchTargets { pub fn all_targets_mut(&mut self) -> &mut [BasicBlock] { &mut self.targets } + + /// Finds the `BasicBlock` to which this `SwitchInt` will branch given the + /// specific value. This cannot fail, as it'll return the `otherwise` + /// branch if there's not a specific match for the value. + pub fn target_for_value(&self, value: u128) -> BasicBlock { + self.iter().find_map(|(v, t)| (v == value).then_some(t)).unwrap_or_else(|| self.otherwise()) + } } pub struct SwitchTargetsIter<'a> { diff --git a/compiler/rustc_mir_transform/src/simplify_branches.rs b/compiler/rustc_mir_transform/src/simplify_branches.rs index df90cfa318df0..d8fc4aa9a6a7f 100644 --- a/compiler/rustc_mir_transform/src/simplify_branches.rs +++ b/compiler/rustc_mir_transform/src/simplify_branches.rs @@ -34,15 +34,8 @@ impl<'tcx> MirPass<'tcx> for SimplifyBranches { } => { let constant = c.literal.try_eval_bits(tcx, param_env, switch_ty); if let Some(constant) = constant { - let otherwise = targets.otherwise(); - let mut ret = TerminatorKind::Goto { target: otherwise }; - for (v, t) in targets.iter() { - if v == constant { - ret = TerminatorKind::Goto { target: t }; - break; - } - } - ret + let target = targets.target_for_value(constant); + TerminatorKind::Goto { target } } else { continue; } diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index b44a339867716..c2e1062cdbbe2 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -623,92 +623,14 @@ impl<'a, 'tcx> MirNeighborCollector<'a, 'tcx> { value, ) } - - fn find_reachable_blocks(&mut self, bb: mir::BasicBlock) { - if !self.reachable_blocks.insert(bb) { - return; - } - - use mir::TerminatorKind::*; - let data = &self.body.basic_blocks()[bb]; - match data.terminator().kind { - Goto { target } => self.find_reachable_blocks(target), - Resume | Abort | Return | Unreachable | GeneratorDrop => (), - Drop { place: _, target, unwind } - | DropAndReplace { place: _, value: _, target, unwind } - | Assert { cond: _, expected: _, msg: _, target, cleanup: unwind } - | Yield { value: _, resume: target, resume_arg: _, drop: unwind } => { - self.find_reachable_blocks(target); - unwind.map(|b| self.find_reachable_blocks(b)); - } - Call { func: _, args: _, destination, cleanup, from_hir_call: _, fn_span: _} => { - destination.map(|(_, b)| self.find_reachable_blocks(b)); - cleanup.map(|b| self.find_reachable_blocks(b)); - } - FalseEdge { .. } | FalseUnwind { .. } => { - bug!("Expected false edges to already be gone when collecting neighbours for {:?}", self.instance); - } - InlineAsm { template: _, operands: _, options: _, line_spans: _, destination} => { - destination.map(|b| self.find_reachable_blocks(b)); - } - SwitchInt { ref discr, switch_ty, ref targets } => { - if let mir::Operand::Constant(constant) = discr { - if let Some(raw_value) = self.try_eval_constant_to_bits(constant, switch_ty) { - // We know what this is going to be, - // so we can ignore all the other blocks. - for (test_value, target) in targets.iter() { - if test_value == raw_value { - return self.find_reachable_blocks(target); - } - } - - return self.find_reachable_blocks(targets.otherwise()); - } - } - - // If it's not a constant or we can't evaluate it, - // then we have to consider them all as reachable. - for &b in targets.all_targets() { - self.find_reachable_blocks(b) - } - } - } - } - - fn try_eval_constant_to_bits(&self, constant: &mir::Constant<'tcx>, ty: Ty<'tcx>) -> Option { - let env = ty::ParamEnv::reveal_all(); - let ct = self.monomorphize(constant.literal); - let value = match ct { - mir::ConstantKind::Val(value, _) => value, - mir::ConstantKind::Ty(ct) => { - match ct.val { - ty::ConstKind::Unevaluated(ct) => self - .tcx - .const_eval_resolve(env, ct, None).ok()?, - ty::ConstKind::Value(value) => value, - other => span_bug!( - constant.span, - "encountered bad ConstKind after monomorphizing: {:?}", - other - ), - } - } - }; - value.try_to_bits_for_ty(self.tcx, env, ty) - } } impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { - fn visit_body(&mut self, body: &mir::Body<'tcx>) { - self.find_reachable_blocks(mir::START_BLOCK); - self.super_body(body); - } - fn visit_basic_block_data(&mut self, block: mir::BasicBlock, data: &mir::BasicBlockData<'tcx>) { if self.reachable_blocks.contains(block) { self.super_basic_block_data(block, data); } else { - debug!("skipping basic block {:?}", block); + debug!("skipping mono-unreachable basic block {:?}", block); } } @@ -1482,15 +1404,9 @@ fn collect_neighbours<'tcx>( debug!("collect_neighbours: {:?}", instance.def_id()); let body = tcx.instance_mir(instance.def); - let reachable_blocks = - if instance.substs.is_noop() { - // If it's non-generic, then it's already filtered out - // any blocks that are unreachable, so don't re-do the work. - BitSet::new_filled(body.basic_blocks().len()) - } else { - BitSet::new_empty(body.basic_blocks().len()) - }; - let mut collector = MirNeighborCollector { tcx, body: &body, output, instance, reachable_blocks }; + let reachable_blocks = body.reachable_blocks_in_mono(tcx, instance); + let mut collector = + MirNeighborCollector { tcx, body: &body, output, instance, reachable_blocks }; collector.visit_body(&body); } diff --git a/src/test/codegen/skip-mono-inside-if-false.rs b/src/test/codegen/skip-mono-inside-if-false.rs index 7f5ed4331ec3d..2557daa91043c 100644 --- a/src/test/codegen/skip-mono-inside-if-false.rs +++ b/src/test/codegen/skip-mono-inside-if-false.rs @@ -7,19 +7,21 @@ pub fn demo_for_i32() { generic_impl::(); } +// Two important things here: +// - We replace the "then" block with `unreachable` to avoid linking problems +// - We neither declare nor define the `big_impl` that said block "calls". + // CHECK-LABEL: ; skip_mono_inside_if_false::generic_impl // CHECK: start: // CHECK-NEXT: br i1 false, label %[[THEN_BRANCH:bb[0-9]+]], label %[[ELSE_BRANCH:bb[0-9]+]] // CHECK: [[ELSE_BRANCH]]: // CHECK-NEXT: call skip_mono_inside_if_false::small_impl // CHECK: [[THEN_BRANCH]]: -// CHECK-NEXT: call skip_mono_inside_if_false::big_impl - -// Then despite there being calls to both of them, only the ones that's used has a definition. -// The other is only forward-declared, and its use will disappear with LLVM's simplifycfg. +// CHECK-NEXT: unreachable +// CHECK-NOT: @_ZN25skip_mono_inside_if_false8big_impl // CHECK: define internal void @_ZN25skip_mono_inside_if_false10small_impl -// CHECK: declare hidden void @_ZN25skip_mono_inside_if_false8big_impl +// CHECK-NOT: @_ZN25skip_mono_inside_if_false8big_impl fn generic_impl() { trait MagicTrait { diff --git a/src/test/mir-opt/const_prop/control-flow-simplification.rs b/src/test/mir-opt/const_prop/control-flow-simplification.rs index 6a0cda92443b9..bcd205e195aa4 100644 --- a/src/test/mir-opt/const_prop/control-flow-simplification.rs +++ b/src/test/mir-opt/const_prop/control-flow-simplification.rs @@ -1,7 +1,7 @@ // compile-flags: -Zmir-opt-level=1 trait HasSize: Sized { - const BYTES:usize = std::mem::size_of::(); + const BYTES: usize = std::mem::size_of::(); } impl HasSize for This{} diff --git a/src/test/mir-opt/simplify_if_generic_constant.rs b/src/test/mir-opt/simplify_if_generic_constant.rs index fb0727207fedf..159802bbe2826 100644 --- a/src/test/mir-opt/simplify_if_generic_constant.rs +++ b/src/test/mir-opt/simplify_if_generic_constant.rs @@ -1,14 +1,14 @@ #![crate_type = "lib"] pub trait HasBoolConst { - const B: bool; + const B: bool; } // EMIT_MIR simplify_if_generic_constant.use_associated_const.SimplifyIfConst.diff pub fn use_associated_const() -> u8 { - if ::B { - 13 - } else { - 42 - } + if ::B { + 13 + } else { + 42 + } } diff --git a/src/test/mir-opt/simplify_if_generic_constant.use_associated_const.SimplifyIfConst.diff b/src/test/mir-opt/simplify_if_generic_constant.use_associated_const.SimplifyIfConst.diff index d9554ba75c923..2a006c541cd2c 100644 --- a/src/test/mir-opt/simplify_if_generic_constant.use_associated_const.SimplifyIfConst.diff +++ b/src/test/mir-opt/simplify_if_generic_constant.use_associated_const.SimplifyIfConst.diff @@ -3,27 +3,27 @@ fn use_associated_const() -> u8 { let mut _0: u8; // return place in scope 0 at $DIR/simplify_if_generic_constant.rs:8:51: 8:53 - let mut _1: bool; // in scope 0 at $DIR/simplify_if_generic_constant.rs:9:5: 9:27 + let mut _1: bool; // in scope 0 at $DIR/simplify_if_generic_constant.rs:9:8: 9:30 bb0: { - StorageLive(_1); // scope 0 at $DIR/simplify_if_generic_constant.rs:9:5: 9:27 -- _1 = const ::B; // scope 0 at $DIR/simplify_if_generic_constant.rs:9:5: 9:27 -- switchInt(move _1) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/simplify_if_generic_constant.rs:9:5: 9:27 -+ switchInt(const ::B) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/simplify_if_generic_constant.rs:9:5: 9:27 + StorageLive(_1); // scope 0 at $DIR/simplify_if_generic_constant.rs:9:8: 9:30 +- _1 = const ::B; // scope 0 at $DIR/simplify_if_generic_constant.rs:9:8: 9:30 +- switchInt(move _1) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/simplify_if_generic_constant.rs:9:8: 9:30 ++ switchInt(const ::B) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/simplify_if_generic_constant.rs:9:8: 9:30 } bb1: { - _0 = const 13_u8; // scope 0 at $DIR/simplify_if_generic_constant.rs:10:3: 10:5 - goto -> bb3; // scope 0 at $DIR/simplify_if_generic_constant.rs:9:2: 13:3 + _0 = const 13_u8; // scope 0 at $DIR/simplify_if_generic_constant.rs:10:9: 10:11 + goto -> bb3; // scope 0 at $DIR/simplify_if_generic_constant.rs:9:5: 13:6 } bb2: { - _0 = const 42_u8; // scope 0 at $DIR/simplify_if_generic_constant.rs:12:3: 12:5 - goto -> bb3; // scope 0 at $DIR/simplify_if_generic_constant.rs:9:2: 13:3 + _0 = const 42_u8; // scope 0 at $DIR/simplify_if_generic_constant.rs:12:9: 12:11 + goto -> bb3; // scope 0 at $DIR/simplify_if_generic_constant.rs:9:5: 13:6 } bb3: { - StorageDead(_1); // scope 0 at $DIR/simplify_if_generic_constant.rs:13:2: 13:3 + StorageDead(_1); // scope 0 at $DIR/simplify_if_generic_constant.rs:13:5: 13:6 return; // scope 0 at $DIR/simplify_if_generic_constant.rs:14:2: 14:2 } } diff --git a/src/test/ui/consts/control-flow/drop-fail.precise.stderr b/src/test/ui/consts/control-flow/drop-fail.precise.stderr index 0b0b2443a4a46..578eee5812a1d 100644 --- a/src/test/ui/consts/control-flow/drop-fail.precise.stderr +++ b/src/test/ui/consts/control-flow/drop-fail.precise.stderr @@ -5,7 +5,7 @@ LL | let x = Some(Vec::new()); | ^ constants cannot evaluate destructors error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/drop-fail.rs:39:9 + --> $DIR/drop-fail.rs:44:9 | LL | let mut tmp = None; | ^^^^^^^ constants cannot evaluate destructors diff --git a/src/test/ui/consts/control-flow/drop-fail.rs b/src/test/ui/consts/control-flow/drop-fail.rs index efa5a11c941e9..ed57e1d6e883d 100644 --- a/src/test/ui/consts/control-flow/drop-fail.rs +++ b/src/test/ui/consts/control-flow/drop-fail.rs @@ -8,13 +8,18 @@ const _: Option> = { let x = Some(Vec::new()); //[stock,precise]~^ ERROR destructors cannot be evaluated at compile-time - if true { + if unknown() { x } else { y } }; +#[inline(never)] +const fn unknown() -> bool { + true +} + // We only clear `NeedsDrop` if a local is moved from in entirely. This is a shortcoming of the // existing analysis. const _: Vec = { diff --git a/src/test/ui/consts/control-flow/drop-fail.stock.stderr b/src/test/ui/consts/control-flow/drop-fail.stock.stderr index 72ca4fa08bc4e..a6b7b61177111 100644 --- a/src/test/ui/consts/control-flow/drop-fail.stock.stderr +++ b/src/test/ui/consts/control-flow/drop-fail.stock.stderr @@ -8,7 +8,7 @@ LL | }; | - value is dropped here error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/drop-fail.rs:21:9 + --> $DIR/drop-fail.rs:26:9 | LL | let vec_tuple = (Vec::new(),); | ^^^^^^^^^ constants cannot evaluate destructors @@ -17,7 +17,7 @@ LL | }; | - value is dropped here error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/drop-fail.rs:29:9 + --> $DIR/drop-fail.rs:34:9 | LL | let x: Result<_, Vec> = Ok(Vec::new()); | ^ constants cannot evaluate destructors @@ -26,7 +26,7 @@ LL | }; | - value is dropped here error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/drop-fail.rs:39:9 + --> $DIR/drop-fail.rs:44:9 | LL | let mut tmp = None; | ^^^^^^^ constants cannot evaluate destructors