Auto merge of rust-lang#131650 - saethlin:post-mono-mir-opts, r=<try>

Add post-mono MIR optimizations

Before this PR, all MIR passes had to operate on polymorphic MIR. Thus any MIR transform maybe unable to determine the type of an argument or local (because it's still generic) or it may be unable to determine which function a Call terminator is calling (because it's still generic).

MIR transforms are a highly maintainable solution to a number of compiler problems, but this polymorphic limitation means that they are cannot solve some of our problems that we'd like them to; the most recent examples that come to mind are rust-lang#134082 which has extra limitations because of the polymorphic inliner, and rust-lang#139088 which is explicitly waiting for post-mono MIR passes to happen.

In addition, the lack of post-mono MIR optimizations means that MIR optimizations just miss out on profitable optimizations, which are so valuable that we've added kludges like rust-lang#121421 (a MIR traversal that you better only run at mono-time).

In addition, rustc_codegen_ssa is riddled with on-the-fly monomorphization and optimization; the logic for these trick that we do in codegen in my experience are hard to maintain, and I would much rather have those implemented in a MIR transform.

So this PR adds a new query `codegen_mir` (the MIR for codegen, not that I like the name). I've then replaced _some_ of the kludges in rustc_codegen_ssa with `PostMono` variants of existing MIR transforms.

I've also un-querified `check_mono_item` and put it at the end of the post-mono pass list. Those checks should be post-mono passes too, but I've tried to keep this PR to a reviewable size. It's easy to imagine lots of other places to use post-mono MIR opts and I want the usefulness of this to be clear while the diff is also manageable.

---

This PR has a perf regression. I've hammered on the perf in a number of ways to get it down to what it is. incr-full builds suffer the most because they need to clone, intern, and cache a monomorphized copy of every MIR body. Things are mixed for every other build scenario. In almost all cases, binary sizes improve.

Author: bors

Cargo.lock

@@ -4135,7 +4135,6 @@ dependencies = [
 name = "rustc_monomorphize"
 version = "0.0.0"
 dependencies = [
- "rustc_abi",
  "rustc_ast",
  "rustc_attr_parsing",
  "rustc_data_structures",

compiler/rustc_codegen_cranelift/src/base.rs

@@ -43,7 +43,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!({
@@ -305,19 +305,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

@@ -414,7 +414,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

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

@@ -126,12 +126,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
     where
         T: Copy + TypeFoldable<TyCtxt<'tcx>>,
     {
-        debug!("monomorphize: self.instance={:?}", self.instance);
-        self.instance.instantiate_mir_and_normalize_erasing_regions(
-            self.cx.tcx(),
-            self.cx.typing_env(),
-            ty::EarlyBinder::bind(value),
-        )
+        value
     }
 }
 
@@ -164,7 +159,7 @@ 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>,
 ) {
@@ -173,7 +168,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     let tcx = cx.tcx();
     let llfn = cx.get_fn(instance);
 
-    let mut mir = tcx.instance_mir(instance.def);
+    let mut mir = tcx.codegen_mir(instance);
 
     let fn_abi = cx.fn_abi_of_instance(instance, ty::List::empty());
     debug!("fn_abi: {:?}", fn_abi);
@@ -243,7 +238,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, 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
@@ -303,20 +299,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 = DenseBitSet::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

@@ -80,7 +80,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, None).collect();
+            let mut rpo: Vec<_> = Postorder::new(&self.basic_blocks, START_BLOCK).collect();
             rpo.reverse();
             rpo
         })

compiler/rustc_middle/src/mir/mod.rs

@@ -33,8 +33,8 @@ use crate::mir::interpret::{AllocRange, Scalar};
 use crate::ty::codec::{TyDecoder, TyEncoder};
 use crate::ty::print::{FmtPrinter, Printer, pretty_print_const, with_no_trimmed_paths};
 use crate::ty::{
-    self, GenericArg, GenericArgsRef, Instance, InstanceKind, List, Ty, TyCtxt, TypeVisitableExt,
-    TypingEnv, UserTypeAnnotationIndex,
+    self, GenericArg, GenericArgsRef, InstanceKind, List, Ty, TyCtxt, TypeVisitableExt, TypingEnv,
+    UserTypeAnnotationIndex,
 };
 
 mod basic_blocks;
@@ -605,74 +605,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
-    /// discriminant 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>| {
-            // FIXME(#132279): what is this, why are we using an empty environment here.
-            let typing_env = ty::TypingEnv::fully_monomorphized();
-            let mono_literal = instance.instantiate_mir_and_normalize_erasing_regions(
-                tcx,
-                typing_env,
-                crate::ty::EarlyBinder::bind(constant.const_),
-            );
-            mono_literal.try_eval_bits(tcx, typing_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,
@@ -1353,19 +1285,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/traversal.rs

@@ -98,21 +98,17 @@ pub struct Postorder<'a, 'tcx> {
     basic_blocks: &'a IndexSlice<BasicBlock, BasicBlockData<'tcx>>,
     visited: DenseBitSet<BasicBlock>,
     visit_stack: Vec<(BasicBlock, Successors<'a>)>,
-    /// A non-empty `extra` allows for a precise calculation of the successors.
-    extra: Option<(TyCtxt<'tcx>, Instance<'tcx>)>,
 }
 
 impl<'a, 'tcx> Postorder<'a, 'tcx> {
     pub fn new(
         basic_blocks: &'a IndexSlice<BasicBlock, BasicBlockData<'tcx>>,
         root: BasicBlock,
-        extra: Option<(TyCtxt<'tcx>, Instance<'tcx>)>,
     ) -> Postorder<'a, 'tcx> {
         let mut po = Postorder {
             basic_blocks,
             visited: DenseBitSet::new_empty(basic_blocks.len()),
             visit_stack: Vec::new(),
-            extra,
         };
 
         po.visit(root);
@@ -126,11 +122,7 @@ impl<'a, 'tcx> Postorder<'a, 'tcx> {
             return;
         }
         let data = &self.basic_blocks[bb];
-        let successors = if let Some(extra) = self.extra {
-            data.mono_successors(extra.0, extra.1)
-        } else {
-            data.terminator().successors()
-        };
+        let successors = data.terminator().successors();
         self.visit_stack.push((bb, successors));
     }
 
@@ -225,20 +217,6 @@ pub fn postorder<'a, 'tcx>(
     reverse_postorder(body).rev()
 }
 
-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, Some((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.
 ///
@@ -286,91 +264,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 [`DenseBitSet`] 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>,
-) -> DenseBitSet<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: DenseBitSet<BasicBlock>,
-    // Other traversers track their worklist in a Vec. But we don't care about order, so we can
-    // store ours in a DenseBitSet and thus save allocations because DenseBitSet has a small size
-    // optimization.
-    worklist: DenseBitSet<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 = DenseBitSet::new_empty(body.basic_blocks.len());
-        worklist.insert(START_BLOCK);
-        MonoReachable {
-            body,
-            tcx,
-            instance,
-            visited: DenseBitSet::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
-    }
-}