Add post-mono MIR passes to make mono-reachable analysis more accurate

compiler/rustc_codegen_cranelift/src/base.rs

@@ -41,7 +41,7 @@ pub(crate) fn codegen_fn<'tcx>(
     let symbol_name = tcx.symbol_name(instance).name.to_string();
     let _timer = tcx.prof.generic_activity_with_arg("codegen fn", &*symbol_name);
 
-    let mir = tcx.instance_mir(instance.def);
+    let mir = tcx.codegen_mir(instance);
     let _mir_guard = crate::PrintOnPanic(|| {
         let mut buf = Vec::new();
         with_no_trimmed_paths!({
@@ -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;

compiler/rustc_codegen_ssa/src/base.rs

@@ -416,7 +416,7 @@ pub(crate) fn codegen_instance<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>>(
     // release builds.
     info!("codegen_instance({})", instance);
 
-    mir::codegen_mir::<Bx>(cx, instance);
+    mir::lower_mir::<Bx>(cx, instance);
 }
 
 /// Creates the `main` function which will initialize the rust runtime and call

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -1252,16 +1252,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,

compiler/rustc_codegen_ssa/src/mir/mod.rs

@@ -159,15 +159,15 @@ impl<'tcx, V: CodegenObject> LocalRef<'tcx, V> {
 ///////////////////////////////////////////////////////////////////////////
 
 #[instrument(level = "debug", skip(cx))]
-pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
+pub fn lower_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     cx: &'a Bx::CodegenCx,
     instance: Instance<'tcx>,
 ) {
     assert!(!instance.args.has_infer());
 
     let llfn = cx.get_fn(instance);
 
-    let mir = cx.tcx().instance_mir(instance.def);
+    let mir = cx.tcx().codegen_mir(instance);
 
     let fn_abi = cx.fn_abi_of_instance(instance, ty::List::empty());
     debug!("fn_abi: {:?}", fn_abi);
@@ -222,7 +222,8 @@ pub fn codegen_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
@@ -282,20 +283,9 @@ pub fn codegen_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);
     }
 }
 

compiler/rustc_middle/src/mir/basic_blocks.rs

@@ -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
         })

compiler/rustc_middle/src/mir/mod.rs

@@ -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,
 };
 
@@ -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,
@@ -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()
-        }
-    }
 }
 
 ///////////////////////////////////////////////////////////////////////////

compiler/rustc_middle/src/mir/mono.rs

@@ -19,6 +19,7 @@ use rustc_target::spec::SymbolVisibility;
 use tracing::debug;
 
 use crate::dep_graph::{DepNode, WorkProduct, WorkProductId};
+use crate::query::Providers;
 use crate::ty::{GenericArgs, Instance, InstanceKind, SymbolName, TyCtxt};
 
 /// Describes how a monomorphization will be instantiated in object files.
@@ -46,7 +47,7 @@ pub enum InstantiationMode {
     LocalCopy,
 }
 
-#[derive(PartialEq, Eq, Clone, Copy, Debug, Hash, HashStable)]
+#[derive(PartialEq, Eq, Clone, Copy, Debug, Hash, HashStable, TyEncodable, TyDecodable)]
 pub enum MonoItem<'tcx> {
     Fn(Instance<'tcx>),
     Static(DefId),
@@ -62,30 +63,6 @@ impl<'tcx> MonoItem<'tcx> {
         }
     }
 
-    // Note: if you change how item size estimates work, you might need to
-    // change NON_INCR_MIN_CGU_SIZE as well.
-    pub fn size_estimate(&self, tcx: TyCtxt<'tcx>) -> usize {
-        match *self {
-            MonoItem::Fn(instance) => {
-                match instance.def {
-                    // "Normal" functions size estimate: the number of
-                    // statements, plus one for the terminator.
-                    InstanceKind::Item(..)
-                    | InstanceKind::DropGlue(..)
-                    | InstanceKind::AsyncDropGlueCtorShim(..) => {
-                        let mir = tcx.instance_mir(instance.def);
-                        mir.basic_blocks.iter().map(|bb| bb.statements.len() + 1).sum()
-                    }
-                    // Other compiler-generated shims size estimate: 1
-                    _ => 1,
-                }
-            }
-            // Conservatively estimate the size of a static declaration or
-            // assembly item to be 1.
-            MonoItem::Static(_) | MonoItem::GlobalAsm(_) => 1,
-        }
-    }
-
     pub fn is_generic_fn(&self) -> bool {
         match self {
             MonoItem::Fn(instance) => instance.args.non_erasable_generics().next().is_some(),
@@ -556,3 +533,49 @@ impl<'tcx> CodegenUnitNameBuilder<'tcx> {
         Symbol::intern(&cgu_name)
     }
 }
+
+/// See module-level docs on some contect for "mentioned" items.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)]
+pub enum CollectionMode {
+    /// Collect items that are used, i.e., actually needed for codegen.
+    ///
+    /// Which items are used can depend on optimization levels, as MIR optimizations can remove
+    /// uses.
+    UsedItems,
+    /// Collect items that are mentioned. The goal of this mode is that it is independent of
+    /// optimizations: the set of "mentioned" items is computed before optimizations are run.
+    ///
+    /// The exact contents of this set are *not* a stable guarantee. (For instance, it is currently
+    /// computed after drop-elaboration. If we ever do some optimizations even in debug builds, we
+    /// might decide to run them before computing mentioned items.) The key property of this set is
+    /// that it is optimization-independent.
+    MentionedItems,
+}
+
+// Note: if you change how item size estimates work, you might need to
+// change NON_INCR_MIN_CGU_SIZE as well.
+fn size_estimate<'tcx>(tcx: TyCtxt<'tcx>, item: MonoItem<'tcx>) -> usize {
+    match item {
+        MonoItem::Fn(instance) => {
+            match instance.def {
+                // "Normal" functions size estimate: the number of
+                // statements, plus one for the terminator.
+                InstanceKind::Item(..)
+                | InstanceKind::DropGlue(..)
+                | InstanceKind::AsyncDropGlueCtorShim(..) => {
+                    let mir = tcx.codegen_mir(instance);
+                    mir.basic_blocks.iter().map(|bb| bb.statements.len() + 1).sum()
+                }
+                // Other compiler-generated shims size estimate: 1
+                _ => 1,
+            }
+        }
+        // Conservatively estimate the size of a static declaration or
+        // assembly item to be 1.
+        MonoItem::Static(_) | MonoItem::GlobalAsm(_) => 1,
+    }
+}
+
+pub fn provide(providers: &mut Providers) {
+    providers.size_estimate = size_estimate;
+}