Auto merge of #85316 - eddyb:cg-ssa-on-demand-cleanuppad, r=nagisa

rustc_codegen_ssa: generate MSVC cleanup pads on demand, like GNU landing pads.

This unblocks #84993 in terms of codegen tests, as it brings the MSVC-style (`cleanup_pad`) EH (LLVM) block order in line with the GNU-style (`landing_pad`) EH (LLVM) block order, by having both of them be on-demand (instead of MSVC-style being eager and GNU-style lazy/on-demand).

It also unifies the two implementations a bit, similar to #84699, but in the opposite direction (as that attempt made both kinds of EH pads eagerly built).

~~Opening as draft because I haven't done enough Windows testing just yet, of both this PR, and of #84993 rebased on it.~~ (**EDIT**: seems to be working as expected)

r? `@nagisa`

Author: bors

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -20,7 +20,7 @@ use rustc_middle::ty::{self, Instance, Ty, TypeFoldable};
 use rustc_span::source_map::Span;
 use rustc_span::{sym, Symbol};
 use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode};
-use rustc_target::abi::{self, LayoutOf};
+use rustc_target::abi::{self, HasDataLayout, LayoutOf};
 use rustc_target::spec::abi::Abi;
 
 /// Used by `FunctionCx::codegen_terminator` for emitting common patterns
@@ -32,13 +32,34 @@ struct TerminatorCodegenHelper<'tcx> {
 }
 
 impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
-    /// Returns the associated funclet from `FunctionCx::funclets` for the
-    /// `funclet_bb` member if it is not `None`.
+    /// Returns the appropriate `Funclet` for the current funclet, if on MSVC,
+    /// either already previously cached, or newly created, by `landing_pad_for`.
     fn funclet<'b, Bx: BuilderMethods<'a, 'tcx>>(
         &self,
-        fx: &'b FunctionCx<'a, 'tcx, Bx>,
+        fx: &'b mut FunctionCx<'a, 'tcx, Bx>,
     ) -> Option<&'b Bx::Funclet> {
-        self.funclet_bb.and_then(|funcl| fx.funclets[funcl].as_ref())
+        let funclet_bb = self.funclet_bb?;
+        if base::wants_msvc_seh(fx.cx.tcx().sess) {
+            // If `landing_pad_for` hasn't been called yet to create the `Funclet`,
+            // it has to be now. This may not seem necessary, as RPO should lead
+            // to all the unwind edges being visited (and so to `landing_pad_for`
+            // getting called for them), before building any of the blocks inside
+            // the funclet itself - however, if MIR contains edges that end up not
+            // being needed in the LLVM IR after monomorphization, the funclet may
+            // be unreachable, and we don't have yet a way to skip building it in
+            // such an eventuality (which may be a better solution than this).
+            if fx.funclets[funclet_bb].is_none() {
+                fx.landing_pad_for(funclet_bb);
+            }
+
+            Some(
+                fx.funclets[funclet_bb]
+                    .as_ref()
+                    .expect("landing_pad_for didn't also create funclets entry"),
+            )
+        } else {
+            None
+        }
     }
 
     fn lltarget<Bx: BuilderMethods<'a, 'tcx>>(
@@ -54,10 +75,10 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
             (Some(f), Some(t_f)) if f == t_f || !base::wants_msvc_seh(fx.cx.tcx().sess) => {
                 (lltarget, false)
             }
-            // jump *into* cleanup - need a landing pad if GNU
-            (None, Some(_)) => (fx.landing_pad_to(target), false),
+            // jump *into* cleanup - need a landing pad if GNU, cleanup pad if MSVC
+            (None, Some(_)) => (fx.landing_pad_for(target), false),
             (Some(_), None) => span_bug!(span, "{:?} - jump out of cleanup?", self.terminator),
-            (Some(_), Some(_)) => (fx.landing_pad_to(target), true),
+            (Some(_), Some(_)) => (fx.landing_pad_for(target), true),
         }
     }
 
@@ -1170,38 +1191,88 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         }
     }
 
-    /// Returns the landing-pad wrapper around the given basic block.
-    ///
-    /// No-op in MSVC SEH scheme.
-    fn landing_pad_to(&mut self, target_bb: mir::BasicBlock) -> Bx::BasicBlock {
-        if let Some(block) = self.landing_pads[target_bb] {
-            return block;
+    /// Returns the landing/cleanup pad wrapper around the given basic block.
+    // FIXME(eddyb) rename this to `eh_pad_for`.
+    fn landing_pad_for(&mut self, bb: mir::BasicBlock) -> Bx::BasicBlock {
+        if let Some(landing_pad) = self.landing_pads[bb] {
+            return landing_pad;
         }
 
-        let block = self.blocks[target_bb];
-        let landing_pad = self.landing_pad_uncached(block);
-        self.landing_pads[target_bb] = Some(landing_pad);
+        let landing_pad = self.landing_pad_for_uncached(bb);
+        self.landing_pads[bb] = Some(landing_pad);
         landing_pad
     }
 
-    fn landing_pad_uncached(&mut self, target_bb: Bx::BasicBlock) -> Bx::BasicBlock {
+    // FIXME(eddyb) rename this to `eh_pad_for_uncached`.
+    fn landing_pad_for_uncached(&mut self, bb: mir::BasicBlock) -> Bx::BasicBlock {
+        let llbb = self.blocks[bb];
         if base::wants_msvc_seh(self.cx.sess()) {
-            span_bug!(self.mir.span, "landing pad was not inserted?")
-        }
-
-        let mut bx = self.new_block("cleanup");
+            let funclet;
+            let ret_llbb;
+            match self.mir[bb].terminator.as_ref().map(|t| &t.kind) {
+                // This is a basic block that we're aborting the program for,
+                // notably in an `extern` function. These basic blocks are inserted
+                // so that we assert that `extern` functions do indeed not panic,
+                // and if they do we abort the process.
+                //
+                // On MSVC these are tricky though (where we're doing funclets). If
+                // we were to do a cleanuppad (like below) the normal functions like
+                // `longjmp` would trigger the abort logic, terminating the
+                // program. Instead we insert the equivalent of `catch(...)` for C++
+                // which magically doesn't trigger when `longjmp` files over this
+                // frame.
+                //
+                // Lots more discussion can be found on #48251 but this codegen is
+                // modeled after clang's for:
+                //
+                //      try {
+                //          foo();
+                //      } catch (...) {
+                //          bar();
+                //      }
+                Some(&mir::TerminatorKind::Abort) => {
+                    let mut cs_bx = self.new_block(&format!("cs_funclet{:?}", bb));
+                    let mut cp_bx = self.new_block(&format!("cp_funclet{:?}", bb));
+                    ret_llbb = cs_bx.llbb();
+
+                    let cs = cs_bx.catch_switch(None, None, 1);
+                    cs_bx.add_handler(cs, cp_bx.llbb());
+
+                    // The "null" here is actually a RTTI type descriptor for the
+                    // C++ personality function, but `catch (...)` has no type so
+                    // it's null. The 64 here is actually a bitfield which
+                    // represents that this is a catch-all block.
+                    let null = cp_bx.const_null(
+                        cp_bx.type_i8p_ext(cp_bx.cx().data_layout().instruction_address_space),
+                    );
+                    let sixty_four = cp_bx.const_i32(64);
+                    funclet = cp_bx.catch_pad(cs, &[null, sixty_four, null]);
+                    cp_bx.br(llbb);
+                }
+                _ => {
+                    let mut cleanup_bx = self.new_block(&format!("funclet_{:?}", bb));
+                    ret_llbb = cleanup_bx.llbb();
+                    funclet = cleanup_bx.cleanup_pad(None, &[]);
+                    cleanup_bx.br(llbb);
+                }
+            }
+            self.funclets[bb] = Some(funclet);
+            ret_llbb
+        } else {
+            let mut bx = self.new_block("cleanup");
 
-        let llpersonality = self.cx.eh_personality();
-        let llretty = self.landing_pad_type();
-        let lp = bx.landing_pad(llretty, llpersonality, 1);
-        bx.set_cleanup(lp);
+            let llpersonality = self.cx.eh_personality();
+            let llretty = self.landing_pad_type();
+            let lp = bx.landing_pad(llretty, llpersonality, 1);
+            bx.set_cleanup(lp);
 
-        let slot = self.get_personality_slot(&mut bx);
-        slot.storage_live(&mut bx);
-        Pair(bx.extract_value(lp, 0), bx.extract_value(lp, 1)).store(&mut bx, slot);
+            let slot = self.get_personality_slot(&mut bx);
+            slot.storage_live(&mut bx);
+            Pair(bx.extract_value(lp, 0), bx.extract_value(lp, 1)).store(&mut bx, slot);
 
-        bx.br(target_bb);
-        bx.llbb()
+            bx.br(llbb);
+            bx.llbb()
+        }
     }
 
     fn landing_pad_type(&self) -> Bx::Type {

compiler/rustc_codegen_ssa/src/mir/mod.rs

@@ -1,19 +1,16 @@
-use crate::base;
 use crate::traits::*;
 use rustc_errors::ErrorReported;
 use rustc_middle::mir;
 use rustc_middle::mir::interpret::ErrorHandled;
 use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt, TyAndLayout};
 use rustc_middle::ty::{self, Instance, Ty, TypeFoldable};
 use rustc_target::abi::call::{FnAbi, PassMode};
-use rustc_target::abi::HasDataLayout;
 
 use std::iter;
 
 use rustc_index::bit_set::BitSet;
 use rustc_index::vec::IndexVec;
 
-use self::analyze::CleanupKind;
 use self::debuginfo::{FunctionDebugContext, PerLocalVarDebugInfo};
 use self::place::PlaceRef;
 use rustc_middle::mir::traversal;
@@ -49,12 +46,13 @@ pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> {
     /// The funclet status of each basic block
     cleanup_kinds: IndexVec<mir::BasicBlock, analyze::CleanupKind>,
 
-    /// When targeting MSVC, this stores the cleanup info for each funclet
-    /// BB. This is initialized as we compute the funclets' head block in RPO.
+    /// When targeting MSVC, this stores the cleanup info for each funclet BB.
+    /// This is initialized at the same time as the `landing_pads` entry for the
+    /// funclets' head block, i.e. when needed by an unwind / `cleanup_ret` edge.
     funclets: IndexVec<mir::BasicBlock, Option<Bx::Funclet>>,
 
-    /// This stores the landing-pad block for a given BB, computed lazily on GNU
-    /// and eagerly on MSVC.
+    /// This stores the cached landing/cleanup pad block for a given BB.
+    // FIXME(eddyb) rename this to `eh_pads`.
     landing_pads: IndexVec<mir::BasicBlock, Option<Bx::BasicBlock>>,
 
     /// Cached unreachable block
@@ -165,7 +163,6 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
         })
         .collect();
 
-    let (landing_pads, funclets) = create_funclets(&mir, &mut bx, &cleanup_kinds, &block_bxs);
     let mut fx = FunctionCx {
         instance,
         mir,
@@ -176,8 +173,8 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
         blocks: block_bxs,
         unreachable_block: None,
         cleanup_kinds,
-        landing_pads,
-        funclets,
+        landing_pads: IndexVec::from_elem(None, mir.basic_blocks()),
+        funclets: IndexVec::from_fn_n(|_| None, mir.basic_blocks().len()),
         locals: IndexVec::new(),
         debug_context,
         per_local_var_debug_info: None,
@@ -273,77 +270,6 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
     }
 }
 
-fn create_funclets<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
-    mir: &'tcx mir::Body<'tcx>,
-    bx: &mut Bx,
-    cleanup_kinds: &IndexVec<mir::BasicBlock, CleanupKind>,
-    block_bxs: &IndexVec<mir::BasicBlock, Bx::BasicBlock>,
-) -> (
-    IndexVec<mir::BasicBlock, Option<Bx::BasicBlock>>,
-    IndexVec<mir::BasicBlock, Option<Bx::Funclet>>,
-) {
-    iter::zip(block_bxs.iter_enumerated(), cleanup_kinds)
-        .map(|((bb, &llbb), cleanup_kind)| {
-            match *cleanup_kind {
-                CleanupKind::Funclet if base::wants_msvc_seh(bx.sess()) => {}
-                _ => return (None, None),
-            }
-
-            let funclet;
-            let ret_llbb;
-            match mir[bb].terminator.as_ref().map(|t| &t.kind) {
-                // This is a basic block that we're aborting the program for,
-                // notably in an `extern` function. These basic blocks are inserted
-                // so that we assert that `extern` functions do indeed not panic,
-                // and if they do we abort the process.
-                //
-                // On MSVC these are tricky though (where we're doing funclets). If
-                // we were to do a cleanuppad (like below) the normal functions like
-                // `longjmp` would trigger the abort logic, terminating the
-                // program. Instead we insert the equivalent of `catch(...)` for C++
-                // which magically doesn't trigger when `longjmp` files over this
-                // frame.
-                //
-                // Lots more discussion can be found on #48251 but this codegen is
-                // modeled after clang's for:
-                //
-                //      try {
-                //          foo();
-                //      } catch (...) {
-                //          bar();
-                //      }
-                Some(&mir::TerminatorKind::Abort) => {
-                    let mut cs_bx = bx.build_sibling_block(&format!("cs_funclet{:?}", bb));
-                    let mut cp_bx = bx.build_sibling_block(&format!("cp_funclet{:?}", bb));
-                    ret_llbb = cs_bx.llbb();
-
-                    let cs = cs_bx.catch_switch(None, None, 1);
-                    cs_bx.add_handler(cs, cp_bx.llbb());
-
-                    // The "null" here is actually a RTTI type descriptor for the
-                    // C++ personality function, but `catch (...)` has no type so
-                    // it's null. The 64 here is actually a bitfield which
-                    // represents that this is a catch-all block.
-                    let null = bx.const_null(
-                        bx.type_i8p_ext(bx.cx().data_layout().instruction_address_space),
-                    );
-                    let sixty_four = bx.const_i32(64);
-                    funclet = cp_bx.catch_pad(cs, &[null, sixty_four, null]);
-                    cp_bx.br(llbb);
-                }
-                _ => {
-                    let mut cleanup_bx = bx.build_sibling_block(&format!("funclet_{:?}", bb));
-                    ret_llbb = cleanup_bx.llbb();
-                    funclet = cleanup_bx.cleanup_pad(None, &[]);
-                    cleanup_bx.br(llbb);
-                }
-            };
-
-            (Some(ret_llbb), Some(funclet))
-        })
-        .unzip()
-}
-
 /// Produces, for each argument, a `Value` pointing at the
 /// argument's value. As arguments are places, these are always
 /// indirect.