Skip to content

Commit

Permalink
Skip those blocks in codegen too, to avoid the linker errors
Browse files Browse the repository at this point in the history
And hey, maybe even be faster because it means less IR emitted.
  • Loading branch information
scottmcm committed Nov 27, 2021
1 parent 10acbf8 commit b5d6de2
Show file tree
Hide file tree
Showing 13 changed files with 132 additions and 127 deletions.
6 changes: 6 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
71 changes: 69 additions & 2 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -517,6 +517,73 @@ impl<'tcx> Body<'tcx> {
pub fn generator_kind(&self) -> Option<GeneratorKind> {
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 <T as Trait>::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<BasicBlock> {
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<BasicBlock>,
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)]
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/mir/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
11 changes: 2 additions & 9 deletions compiler/rustc_mir_transform/src/simplify_branches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
92 changes: 4 additions & 88 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u128> {
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);
}
}

Expand Down Expand Up @@ -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);
}

Expand Down
12 changes: 7 additions & 5 deletions src/test/codegen/skip-mono-inside-if-false.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,21 @@ pub fn demo_for_i32() {
generic_impl::<i32>();
}

// 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<T>() {
trait MagicTrait {
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/const_prop/control-flow-simplification.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// compile-flags: -Zmir-opt-level=1

trait HasSize: Sized {
const BYTES:usize = std::mem::size_of::<Self>();
const BYTES: usize = std::mem::size_of::<Self>();
}

impl<This> HasSize for This{}
Expand Down
12 changes: 6 additions & 6 deletions src/test/mir-opt/simplify_if_generic_constant.rs
Original file line number Diff line number Diff line change
@@ -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<T: HasBoolConst>() -> u8 {
if <T as HasBoolConst>::B {
13
} else {
42
}
if <T as HasBoolConst>::B {
13
} else {
42
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 <T as HasBoolConst>::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 <T as HasBoolConst>::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 <T as HasBoolConst>::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 <T as HasBoolConst>::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
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/consts/control-flow/drop-fail.precise.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion src/test/ui/consts/control-flow/drop-fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,18 @@ const _: Option<Vec<i32>> = {
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<i32> = {
Expand Down
Loading

0 comments on commit b5d6de2

Please sign in to comment.