Apply RemoveNoopLandingPads post-monomorphization

Cargo.lock

@@ -3887,6 +3887,7 @@ dependencies = [
  "rustc_macros",
  "rustc_metadata",
  "rustc_middle",
+ "rustc_mir_transform",
  "rustc_serialize",
  "rustc_session",
  "rustc_span",

compiler/rustc_codegen_ssa/Cargo.toml

@@ -27,6 +27,7 @@ rustc_lint_defs = { path = "../rustc_lint_defs" }
 rustc_macros = { path = "../rustc_macros" }
 rustc_metadata = { path = "../rustc_metadata" }
 rustc_middle = { path = "../rustc_middle" }
+rustc_mir_transform = { path = "../rustc_mir_transform" }
 rustc_serialize = { path = "../rustc_serialize" }
 rustc_session = { path = "../rustc_session" }
 rustc_span = { path = "../rustc_span" }

compiler/rustc_codegen_ssa/src/mir/analyze.rs

@@ -292,10 +292,14 @@ impl CleanupKind {
 /// MSVC requires unwinding code to be split to a tree of *funclets*, where each funclet can only
 /// branch to itself or to its parent. Luckily, the code we generates matches this pattern.
 /// Recover that structure in an analyze pass.
-pub(crate) fn cleanup_kinds(mir: &mir::Body<'_>) -> IndexVec<mir::BasicBlock, CleanupKind> {
+pub(crate) fn cleanup_kinds(
+    mir: &mir::Body<'_>,
+    nop_landing_pads: &DenseBitSet<mir::BasicBlock>,
+) -> IndexVec<mir::BasicBlock, CleanupKind> {
     fn discover_masters<'tcx>(
         result: &mut IndexSlice<mir::BasicBlock, CleanupKind>,
         mir: &mir::Body<'tcx>,
+        nop_landing_pads: &DenseBitSet<mir::BasicBlock>,
     ) {
         for (bb, data) in mir.basic_blocks.iter_enumerated() {
             match data.terminator().kind {
@@ -314,7 +318,9 @@ pub(crate) fn cleanup_kinds(mir: &mir::Body<'_>) -> IndexVec<mir::BasicBlock, Cl
                 | TerminatorKind::InlineAsm { unwind, .. }
                 | TerminatorKind::Assert { unwind, .. }
                 | TerminatorKind::Drop { unwind, .. } => {
-                    if let mir::UnwindAction::Cleanup(unwind) = unwind {
+                    if let mir::UnwindAction::Cleanup(unwind) = unwind
+                        && !nop_landing_pads.contains(unwind)
+                    {
                         debug!(
                             "cleanup_kinds: {:?}/{:?} registering {:?} as funclet",
                             bb, data, unwind
@@ -395,7 +401,7 @@ pub(crate) fn cleanup_kinds(mir: &mir::Body<'_>) -> IndexVec<mir::BasicBlock, Cl
 
     let mut result = IndexVec::from_elem(CleanupKind::NotCleanup, &mir.basic_blocks);
 
-    discover_masters(&mut result, mir);
+    discover_masters(&mut result, mir, &nop_landing_pads);
     propagate(&mut result, mir);
     debug!("cleanup_kinds: result={:?}", result);
     result

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -216,7 +216,13 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
         }
 
         let unwind_block = match unwind {
-            mir::UnwindAction::Cleanup(cleanup) => Some(self.llbb_with_cleanup(fx, cleanup)),
+            mir::UnwindAction::Cleanup(cleanup) => {
+                if !fx.nop_landing_pads.contains(cleanup) {
+                    Some(self.llbb_with_cleanup(fx, cleanup))
+                } else {
+                    None
+                }
+            }
             mir::UnwindAction::Continue => None,
             mir::UnwindAction::Unreachable => None,
             mir::UnwindAction::Terminate(reason) => {
@@ -319,7 +325,13 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
         mergeable_succ: bool,
     ) -> MergingSucc {
         let unwind_target = match unwind {
-            mir::UnwindAction::Cleanup(cleanup) => Some(self.llbb_with_cleanup(fx, cleanup)),
+            mir::UnwindAction::Cleanup(cleanup) => {
+                if !fx.nop_landing_pads.contains(cleanup) {
+                    Some(self.llbb_with_cleanup(fx, cleanup))
+                } else {
+                    None
+                }
+            }
             mir::UnwindAction::Terminate(reason) => Some(fx.terminate_block(reason, None)),
             mir::UnwindAction::Continue => None,
             mir::UnwindAction::Unreachable => None,

compiler/rustc_codegen_ssa/src/mir/mod.rs

@@ -102,6 +102,8 @@ pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> {
     /// A cold block is a block that is unlikely to be executed at runtime.
     cold_blocks: IndexVec<mir::BasicBlock, bool>,
 
+    nop_landing_pads: DenseBitSet<mir::BasicBlock>,
+
     /// The location where each MIR arg/var/tmp/ret is stored. This is
     /// usually an `PlaceRef` representing an alloca, but not always:
     /// sometimes we can skip the alloca and just store the value
@@ -215,6 +217,15 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     let fn_abi = cx.fn_abi_of_instance(instance, ty::List::empty());
     debug!("fn_abi: {:?}", fn_abi);
 
+    let nop_landing_pads = rustc_mir_transform::remove_noop_landing_pads::find_noop_landing_pads(
+        mir,
+        Some(rustc_mir_transform::remove_noop_landing_pads::ExtraInfo {
+            tcx,
+            instance,
+            typing_env: cx.typing_env(),
+        }),
+    );
+
     if tcx.features().ergonomic_clones() {
         let monomorphized_mir = instance.instantiate_mir_and_normalize_erasing_regions(
             tcx,
@@ -227,14 +238,15 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     let start_llbb = Bx::append_block(cx, llfn, "start");
     let mut start_bx = Bx::build(cx, start_llbb);
 
-    if mir.basic_blocks.iter().any(|bb| {
-        bb.is_cleanup || matches!(bb.terminator().unwind(), Some(mir::UnwindAction::Terminate(_)))
+    if mir::traversal::mono_reachable(&mir, tcx, instance).any(|(bb, block)| {
+        (block.is_cleanup && !nop_landing_pads.contains(bb))
+            || matches!(block.terminator().unwind(), Some(mir::UnwindAction::Terminate(_)))
     }) {
         start_bx.set_personality_fn(cx.eh_personality());
     }
 
-    let cleanup_kinds =
-        base::wants_new_eh_instructions(tcx.sess).then(|| analyze::cleanup_kinds(&mir));
+    let cleanup_kinds = base::wants_new_eh_instructions(tcx.sess)
+        .then(|| analyze::cleanup_kinds(&mir, &nop_landing_pads));
 
     let cached_llbbs: IndexVec<mir::BasicBlock, CachedLlbb<Bx::BasicBlock>> =
         mir.basic_blocks
@@ -262,6 +274,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
         debug_context: None,
         per_local_var_debug_info: None,
         caller_location: None,
+        nop_landing_pads,
     };
 
     // It may seem like we should iterate over `required_consts` to ensure they all successfully
@@ -275,7 +288,36 @@ 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 mut traversal_order = traversal::mono_reachable_reverse_postorder(mir, tcx, instance);
+
+    // Filter out blocks that won't be codegen'd because of nop_landing_pads optimization.
+    // FIXME: We might want to integrate the nop_landing_pads analysis into mono reachability.
+    {
+        let mut reachable = DenseBitSet::new_empty(mir.basic_blocks.len());
+        let mut to_visit = vec![mir::START_BLOCK];
+        while let Some(next) = to_visit.pop() {
+            if !reachable.insert(next) {
+                continue;
+            }
+
+            let block = &mir.basic_blocks[next];
+            if let Some(mir::UnwindAction::Cleanup(target)) = block.terminator().unwind()
+                && fx.nop_landing_pads.contains(*target)
+            {
+                // This edge will not be followed when we actually codegen, so skip generating it here.
+                //
+                // It's guaranteed that the cleanup block (`target`) occurs only in
+                // UnwindAction::Cleanup(...) -- i.e., we can't incorrectly filter too much here --
+                // because cleanup transitions must happen via UnwindAction::Cleanup.
+                to_visit.extend(block.terminator().successors().filter(|s| s != target));
+            } else {
+                to_visit.extend(block.terminator().successors());
+            }
+        }
+
+        traversal_order.retain(|bb| reachable.contains(*bb));
+    }
+
     let memory_locals = analyze::non_ssa_locals(&fx, &traversal_order);
 
     // Allocate variable and temp allocas

compiler/rustc_mir_transform/src/lib.rs

@@ -169,7 +169,7 @@ declare_passes! {
     mod prettify : ReorderBasicBlocks, ReorderLocals;
     mod promote_consts : PromoteTemps;
     mod ref_prop : ReferencePropagation;
-    mod remove_noop_landing_pads : RemoveNoopLandingPads;
+    pub mod remove_noop_landing_pads : RemoveNoopLandingPads;
     mod remove_place_mention : RemovePlaceMention;
     mod remove_storage_markers : RemoveStorageMarkers;
     mod remove_uninit_drops : RemoveUninitDrops;

compiler/rustc_mir_transform/src/remove_noop_landing_pads.rs

@@ -1,6 +1,6 @@
 use rustc_index::bit_set::DenseBitSet;
 use rustc_middle::mir::*;
-use rustc_middle::ty::TyCtxt;
+use rustc_middle::ty::{self, Instance, TyCtxt};
 use tracing::{debug, instrument};
 
 use crate::patch::MirPatch;
@@ -30,17 +30,7 @@ impl<'tcx> crate::MirPass<'tcx> for RemoveNoopLandingPads {
             return;
         }
 
-        let mut nop_landing_pads = DenseBitSet::new_empty(body.basic_blocks.len());
-
-        // This is a post-order traversal, so that if A post-dominates B
-        // then A will be visited before B.
-        for (bb, bbdata) in traversal::postorder(body) {
-            let is_nop_landing_pad = self.is_nop_landing_pad(bbdata, &nop_landing_pads);
-            debug!("is_nop_landing_pad({bb:?}) = {is_nop_landing_pad}");
-            if is_nop_landing_pad {
-                nop_landing_pads.insert(bb);
-            }
-        }
+        let nop_landing_pads = find_noop_landing_pads(body, None);
 
         if nop_landing_pads.is_empty() {
             debug!("no nop landing pads in MIR");
@@ -83,10 +73,12 @@ impl<'tcx> crate::MirPass<'tcx> for RemoveNoopLandingPads {
 }
 
 impl RemoveNoopLandingPads {
-    fn is_nop_landing_pad(
+    fn is_nop_landing_pad<'tcx>(
         &self,
-        bbdata: &BasicBlockData<'_>,
+        bbdata: &BasicBlockData<'tcx>,
+        body: &Body<'tcx>,
         nop_landing_pads: &DenseBitSet<BasicBlock>,
+        extra: Option<&ExtraInfo<'tcx>>,
     ) -> bool {
         for stmt in &bbdata.statements {
             match &stmt.kind {
@@ -128,6 +120,25 @@ impl RemoveNoopLandingPads {
             | TerminatorKind::FalseUnwind { .. } => {
                 terminator.successors().all(|succ| nop_landing_pads.contains(succ))
             }
+            TerminatorKind::Drop { place, .. } => {
+                if let Some(extra) = extra {
+                    let ty = place.ty(body, extra.tcx).ty;
+                    debug!("monomorphize: instance={:?}", extra.instance);
+                    let ty = extra.instance.instantiate_mir_and_normalize_erasing_regions(
+                        extra.tcx,
+                        extra.typing_env,
+                        ty::EarlyBinder::bind(extra.tcx, ty),
+                    );
+                    let drop_fn = Instance::resolve_drop_glue(extra.tcx, ty);
+                    if let ty::InstanceKind::Shim(ty::ShimKind::DropGlue(_, None)) = drop_fn.def {
+                        // no need to drop anything, if all of our successors are also no-op then we
+                        // can be skipped.
+                        return terminator.successors().all(|succ| nop_landing_pads.contains(succ));
+                    }
+                }
+
+                false
+            }
             TerminatorKind::CoroutineDrop
             | TerminatorKind::Yield { .. }
             | TerminatorKind::Return
@@ -136,8 +147,41 @@ impl RemoveNoopLandingPads {
             | TerminatorKind::Call { .. }
             | TerminatorKind::TailCall { .. }
             | TerminatorKind::Assert { .. }
-            | TerminatorKind::Drop { .. }
             | TerminatorKind::InlineAsm { .. } => false,
         }
     }
 }
+
+/// This provides extra information that allows further analysis.
+///
+/// Used by rustc_codegen_ssa.
+pub struct ExtraInfo<'tcx> {
+    pub tcx: TyCtxt<'tcx>,
+    pub instance: Instance<'tcx>,
+    pub typing_env: ty::TypingEnv<'tcx>,
+}
+
+pub fn find_noop_landing_pads<'tcx>(
+    body: &Body<'tcx>,
+    extra: Option<ExtraInfo<'tcx>>,
+) -> DenseBitSet<BasicBlock> {
+    let mut nop_landing_pads = DenseBitSet::new_empty(body.basic_blocks.len());
+
+    // This is a post-order traversal, so that if A post-dominates B
+    // then A will be visited before B.
+    let postorder: Vec<_> = traversal::postorder(body).map(|(bb, _)| bb).collect();
+    for bb in postorder {
+        let is_nop_landing_pad = RemoveNoopLandingPads.is_nop_landing_pad(
+            &body.basic_blocks[bb],
+            body,
+            &nop_landing_pads,
+            extra.as_ref(),
+        );
+        if is_nop_landing_pad {
+            nop_landing_pads.insert(bb);
+        }
+        debug!("    is_nop_landing_pad({:?}) = {}", bb, is_nop_landing_pad);
+    }
+
+    nop_landing_pads
+}

tests/codegen-llvm/unused-drop-pre-llvm.rs

@@ -0,0 +1,27 @@
+//@ needs-unwind - depends on landing pads being optimized away, so not useful to run without it
+//@ compile-flags: -C no-prepopulate-passes
+
+#![crate_type = "lib"]
+
+#[inline(never)]
+fn inner(_: &dyn Sync) {}
+
+fn wrapper<T: Sync>(val: T) {
+    inner(&val);
+}
+
+// Verify that there are no landing pads produced.
+// CHECK-LABEL: unused_drop_pre_llvm::wrapper::<u32>
+// CHECk-NOT: resume
+// CHECk-NOT: landingpad
+// The next line checks for the } that ends the function definition
+// CHECK-LABEL: {{^[}]}}
+#[inline(never)]
+pub fn wrapper_u32() {
+    wrapper(1u32);
+}
+
+#[inline(never)]
+pub fn wrapper_u32_manual(x: u32) {
+    inner(&x);
+}

tests/ui/backtrace/line-tables-only.rs

@@ -45,10 +45,11 @@ fn main() {
 
     // FIXME(jieyouxu): for some forsaken reason on i686-msvc `foo` doesn't have an entry in the
     // line tables?
+    // And with #143208 we also lost `bar` in the line tables.
     #[cfg(not(all(target_pointer_width = "32", target_env = "msvc")))]
     {
         assert_contains(&backtrace, "foo", "line-tables-only-helper.rs", 5);
+        assert_contains(&backtrace, "bar", "line-tables-only-helper.rs", 10);
     }
-    assert_contains(&backtrace, "bar", "line-tables-only-helper.rs", 10);
     assert_contains(&backtrace, "baz", "line-tables-only-helper.rs", 5);
 }