Auto merge of #130909 - saethlin:infer-nounwind, r=<try>

Infer nounwind and use it in MIR opts r? `@ghost` Sinking this into `layout::fn_can_unwind` yields bigger MIR diffs. That's something to try tweaking. But also, this analysis reduces incrementality because call sites depend on callee bodies. So I've currently disabled it when incremental is enabled. That's another tweak I want to try.
rust-lang · Dec 9, 2024 · cc9057d · cc9057d
2 parents f6cb952 + 5bf6851
commit cc9057d
Show file tree

Hide file tree

Showing 28 changed files with 109 additions and 46 deletions.
diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs
@@ -870,7 +870,7 @@ fn run_required_analyses(tcx: TyCtxt<'_>) {
     });
     sess.time("MIR_effect_checking", || {
         for def_id in tcx.hir().body_owners() {
-            tcx.ensure().has_ffi_unwind_calls(def_id);
+            tcx.ensure().mir_flags(def_id);
 
             // If we need to codegen, ensure that we emit all errors from
             // `mir_drops_elaborated_and_const_checked` now, to avoid discovering

diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
@@ -348,6 +348,7 @@ provide! { tcx, def_id, other, cdata,
     is_mir_available => { cdata.is_item_mir_available(def_id.index) }
     is_ctfe_mir_available => { cdata.is_ctfe_mir_available(def_id.index) }
     cross_crate_inlinable => { table_direct }
+    mir_flags => { table_direct }
 
     dylib_dependency_formats => { cdata.get_dylib_dependency_formats(tcx) }
     is_private_dep => { cdata.private_dep }

diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs
@@ -25,6 +25,7 @@ use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile;
 use rustc_middle::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo};
 use rustc_middle::middle::lib_features::FeatureStability;
 use rustc_middle::middle::resolve_bound_vars::ObjectLifetimeDefault;
+use rustc_middle::mir::MirFlags;
 use rustc_middle::ty::fast_reject::SimplifiedType;
 use rustc_middle::ty::{
     self, DeducedParamAttrs, ParameterizedOverTcx, Ty, TyCtxt, UnusedGenericParams,
@@ -401,6 +402,7 @@ define_tables! {
     // individually instead of `DefId`s.
     module_children_reexports: Table<DefIndex, LazyArray<ModChild>>,
     cross_crate_inlinable: Table<DefIndex, bool>,
+    mir_flags: Table<DefIndex, MirFlags>,
 
 - optional:
     attributes: Table<DefIndex, LazyArray<ast::Attribute>>,

diff --git a/compiler/rustc_metadata/src/rmeta/table.rs b/compiler/rustc_metadata/src/rmeta/table.rs
@@ -53,6 +53,12 @@ impl IsDefault for UnusedGenericParams {
     }
 }
 
+impl IsDefault for MirFlags {
+    fn is_default(&self) -> bool {
+        *self == Self::default()
+    }
+}
+
 /// Helper trait, for encoding to, and decoding from, a fixed number of bytes.
 /// Used mainly for Lazy positions and lengths.
 /// Unchecked invariant: `Self::default()` should encode as `[0; BYTE_LEN]`,
@@ -291,6 +297,21 @@ impl FixedSizeEncoding for AttrFlags {
     }
 }
 
+impl FixedSizeEncoding for MirFlags {
+    type ByteArray = [u8; 1];
+
+    #[inline]
+    fn from_bytes(b: &[u8; 1]) -> Self {
+        MirFlags::from_bits_truncate(b[0])
+    }
+
+    #[inline]
+    fn write_to_bytes(self, b: &mut [u8; 1]) {
+        debug_assert!(!self.is_default());
+        b[0] = self.bits();
+    }
+}
+
 impl FixedSizeEncoding for bool {
     type ByteArray = [u8; 1];
 

diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs
@@ -1793,6 +1793,15 @@ impl DefLocation {
     }
 }
 
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, TyEncodable, TyDecodable, HashStable)]
+pub struct MirFlags(u8);
+bitflags::bitflags! {
+    impl MirFlags: u8 {
+        const IS_NOUNWIND = 1 << 0;
+        const HAS_FFI_UNWIND_CALLS = 1 << 1;
+    }
+}
+
 // Some nodes are used a lot. Make sure they don't unintentionally get bigger.
 #[cfg(target_pointer_width = "64")]
 mod size_asserts {

diff --git a/compiler/rustc_middle/src/query/erase.rs b/compiler/rustc_middle/src/query/erase.rs
@@ -293,6 +293,7 @@ trivial! {
     rustc_middle::middle::resolve_bound_vars::ResolvedArg,
     rustc_middle::middle::stability::DeprecationEntry,
     rustc_middle::mir::ConstQualifs,
+    rustc_middle::mir::MirFlags,
     rustc_middle::mir::interpret::AllocId,
     rustc_middle::mir::interpret::CtfeProvenance,
     rustc_middle::mir::interpret::ErrorHandled,

diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs
@@ -56,6 +56,7 @@ use crate::middle::lib_features::LibFeatures;
 use crate::middle::privacy::EffectiveVisibilities;
 use crate::middle::resolve_bound_vars::{ObjectLifetimeDefault, ResolveBoundVars, ResolvedArg};
 use crate::middle::stability::{self, DeprecationEntry};
+use crate::mir::MirFlags;
 use crate::mir::interpret::{
     EvalStaticInitializerRawResult, EvalToAllocationRawResult, EvalToConstValueResult,
     EvalToValTreeResult, GlobalId, LitToConstError, LitToConstInput,
@@ -1547,9 +1548,10 @@ rustc_queries! {
         desc { "checking if a crate is `#![profiler_runtime]`" }
         separate_provide_extern
     }
-    query has_ffi_unwind_calls(key: LocalDefId) -> bool {
-        desc { |tcx| "checking if `{}` contains FFI-unwind calls", tcx.def_path_str(key) }
-        cache_on_disk_if { true }
+    query mir_flags(key: DefId) -> MirFlags {
+        desc { |tcx| "stashing some local properties of `{}` before the body is stolen", tcx.def_path_str(key) }
+        cache_on_disk_if { key.is_local() }
+        separate_provide_extern
     }
     query required_panic_strategy(_: CrateNum) -> Option<PanicStrategy> {
         fatal_cycle

diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs
@@ -64,7 +64,7 @@ use crate::metadata::ModChild;
 use crate::middle::codegen_fn_attrs::CodegenFnAttrs;
 use crate::middle::{resolve_bound_vars, stability};
 use crate::mir::interpret::{self, Allocation, ConstAllocation};
-use crate::mir::{Body, Local, Place, PlaceElem, ProjectionKind, Promoted};
+use crate::mir::{Body, Local, MirFlags, Place, PlaceElem, ProjectionKind, Promoted};
 use crate::query::plumbing::QuerySystem;
 use crate::query::{IntoQueryParam, LocalCrate, Providers, TyCtxtAt};
 use crate::thir::Thir;
@@ -3206,6 +3206,14 @@ impl<'tcx> TyCtxt<'tcx> {
         }
     }
 
+    pub fn is_nounwind(self, def_id: DefId) -> bool {
+        self.mir_flags(def_id).contains(MirFlags::IS_NOUNWIND)
+    }
+
+    pub fn has_ffi_unwind_calls(self, def_id: DefId) -> bool {
+        self.mir_flags(def_id).contains(MirFlags::HAS_FFI_UNWIND_CALLS)
+    }
+
     /// Whether this is a trait implementation that has `#[diagnostic::do_not_recommend]`
     pub fn do_not_recommend_impl(self, def_id: DefId) -> bool {
         self.get_diagnostic_attr(def_id, sym::do_not_recommend).is_some()

diff --git a/compiler/rustc_middle/src/ty/parameterized.rs b/compiler/rustc_middle/src/ty/parameterized.rs
@@ -98,6 +98,7 @@ trivially_parameterized_over_tcx! {
     rustc_hir::OpaqueTyOrigin<rustc_hir::def_id::DefId>,
     rustc_index::bit_set::BitSet<u32>,
     rustc_index::bit_set::FiniteBitSet<u32>,
+    rustc_middle::mir::MirFlags,
     rustc_session::cstore::ForeignModule,
     rustc_session::cstore::LinkagePreference,
     rustc_session::cstore::NativeLib,

diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs
@@ -1,5 +1,5 @@
 use rustc_abi::ExternAbi;
-use rustc_hir::def_id::{LOCAL_CRATE, LocalDefId};
+use rustc_hir::def_id::LOCAL_CRATE;
 use rustc_middle::mir::*;
 use rustc_middle::query::{LocalCrate, Providers};
 use rustc_middle::ty::{self, TyCtxt, layout};
@@ -11,17 +11,10 @@ use tracing::debug;
 use crate::errors;
 
 // Check if the body of this def_id can possibly leak a foreign unwind into Rust code.
-fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool {
-    debug!("has_ffi_unwind_calls({local_def_id:?})");
+pub(crate) fn has_ffi_unwind_calls<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> bool {
+    let def_id = body.source.def_id();
 
-    // Only perform check on functions because constants cannot call FFI functions.
-    let def_id = local_def_id.to_def_id();
-    let kind = tcx.def_kind(def_id);
-    if !kind.is_fn_like() {
-        return false;
-    }
-
-    let body = &*tcx.mir_built(local_def_id).borrow();
+    debug!("has_ffi_unwind_calls({def_id:?})");
 
     let body_ty = tcx.type_of(def_id).skip_binder();
     let body_abi = match body_ty.kind() {
@@ -112,7 +105,7 @@ fn required_panic_strategy(tcx: TyCtxt<'_>, _: LocalCrate) -> Option<PanicStrate
     }
 
     for def_id in tcx.hir().body_owners() {
-        if tcx.has_ffi_unwind_calls(def_id) {
+        if tcx.has_ffi_unwind_calls(def_id.into()) {
             // Given that this crate is compiled in `-C panic=unwind`, the `AbortUnwindingCalls`
             // MIR pass will not be run on FFI-unwind call sites, therefore a foreign exception
             // can enter Rust through these sites.
@@ -143,5 +136,5 @@ fn required_panic_strategy(tcx: TyCtxt<'_>, _: LocalCrate) -> Option<PanicStrate
 }
 
 pub(crate) fn provide(providers: &mut Providers) {
-    *providers = Providers { has_ffi_unwind_calls, required_panic_strategy, ..*providers };
+    *providers = Providers { required_panic_strategy, ..*providers };
 }
diff --git a/compiler/rustc_mir_transform/src/instsimplify.rs b/compiler/rustc_mir_transform/src/instsimplify.rs
@@ -283,7 +283,9 @@ impl<'tcx> InstSimplifyContext<'_, 'tcx> {
             _ => bug!("unexpected body ty: {:?}", body_ty),
         };
 
-        if !layout::fn_can_unwind(self.tcx, Some(def_id), body_abi) {
+        if !layout::fn_can_unwind(self.tcx, Some(def_id), body_abi)
+            || (self.tcx.sess.opts.incremental.is_none() && self.tcx.is_nounwind(def_id))
+        {
             *unwind = UnwindAction::Unreachable;
         }
     }

diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs
@@ -26,8 +26,8 @@ use rustc_hir::def_id::LocalDefId;
 use rustc_index::IndexVec;
 use rustc_middle::mir::{
     AnalysisPhase, Body, CallSource, ClearCrossCrate, ConstOperand, ConstQualifs, LocalDecl,
-    MirPhase, Operand, Place, ProjectionElem, Promoted, RuntimePhase, Rvalue, START_BLOCK,
-    SourceInfo, Statement, StatementKind, TerminatorKind,
+    MirFlags, MirPhase, Operand, Place, ProjectionElem, Promoted, RuntimePhase, Rvalue,
+    START_BLOCK, SourceInfo, Statement, StatementKind, TerminatorKind,
 };
 use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt};
 use rustc_middle::util::Providers;
@@ -217,6 +217,7 @@ pub fn provide(providers: &mut Providers) {
         promoted_mir,
         deduced_param_attrs: deduce_param_attrs::deduced_param_attrs,
         coroutine_by_move_body_def_id: coroutine::coroutine_by_move_body_def_id,
+        mir_flags,
         ..providers.queries
     };
 }
@@ -410,9 +411,6 @@ fn mir_promoted(
         _ => ConstQualifs::default(),
     };
 
-    // the `has_ffi_unwind_calls` query uses the raw mir, so make sure it is run.
-    tcx.ensure_with_value().has_ffi_unwind_calls(def);
-
     // the `by_move_body` query uses the raw mir, so make sure it is run.
     if tcx.needs_coroutine_by_move_body_def_id(def.to_def_id()) {
         tcx.ensure_with_value().coroutine_by_move_body_def_id(def);
@@ -440,6 +438,33 @@ fn mir_promoted(
     (tcx.alloc_steal_mir(body), tcx.alloc_steal_promoted(promoted))
 }
 
+fn mir_flags<'tcx>(tcx: TyCtxt<'tcx>, local_def_id: LocalDefId) -> MirFlags {
+    let mut flags = MirFlags::default();
+    // Only perform check on functions because constants cannot call FFI functions.
+    let kind = tcx.def_kind(local_def_id);
+    if !kind.is_fn_like() {
+        return flags;
+    }
+
+    if !tcx.mir_keys(()).contains(&local_def_id) {
+        return flags;
+    }
+
+    let body = &*tcx.mir_promoted(local_def_id).0.borrow();
+
+    if is_nounwind(body) {
+        flags.insert(MirFlags::IS_NOUNWIND);
+    }
+    if ffi_unwind_calls::has_ffi_unwind_calls(tcx, body) {
+        flags.insert(MirFlags::HAS_FFI_UNWIND_CALLS);
+    }
+    flags
+}
+
+fn is_nounwind<'tcx>(body: &Body<'tcx>) -> bool {
+    body.basic_blocks.iter().all(|block| block.terminator().unwind().is_none())
+}
+
 /// Compute the MIR that is used during CTFE (and thus has no optimizations run on it)
 fn mir_for_ctfe(tcx: TyCtxt<'_>, def_id: LocalDefId) -> &Body<'_> {
     tcx.arena.alloc(inner_mir_for_ctfe(tcx, def_id))
@@ -488,6 +513,7 @@ fn mir_drops_elaborated_and_const_checked(tcx: TyCtxt<'_>, def: LocalDefId) -> &
         if pm::should_run_pass(tcx, &inline::Inline) {
             tcx.ensure_with_value().mir_inliner_callees(ty::InstanceKind::Item(def.to_def_id()));
         }
+        tcx.ensure_with_value().mir_flags(def);
     }
 
     let (body, _) = tcx.mir_promoted(def);

diff --git a/tests/codegen/drop.rs b/tests/codegen/drop.rs
@@ -23,7 +23,7 @@ pub fn droppy() {
     // FIXME(eddyb) the `void @` forces a match on the instruction, instead of the
     // comment, that's `; call core::ptr::drop_in_place::<drop::SomeUniqueName>`
     // for the `v0` mangling, should switch to matching on that once `legacy` is gone.
-    // CHECK-COUNT-6: {{(call|invoke) void @.*}}drop_in_place{{.*}}SomeUniqueName
+    // CHECK-COUNT-5: {{(call|invoke) void @.*}}drop_in_place{{.*}}SomeUniqueName
     // CHECK-NOT: {{(call|invoke) void @.*}}drop_in_place{{.*}}SomeUniqueName
     // The next line checks for the } that ends the function definition
     // CHECK-LABEL: {{^[}]}}

diff --git a/tests/codegen/personality_lifetimes.rs b/tests/codegen/personality_lifetimes.rs
@@ -13,7 +13,9 @@ impl Drop for S {
 }
 
 #[inline(never)]
-fn might_unwind() {}
+fn might_unwind() {
+    panic!()
+}
 
 // CHECK-LABEL: @test
 #[no_mangle]

diff --git a/tests/mir-opt/inline/caller_with_trivial_bound.foo.Inline.panic-unwind.diff b/tests/mir-opt/inline/caller_with_trivial_bound.foo.Inline.panic-unwind.diff
@@ -10,7 +10,7 @@
 
       bb0: {
           StorageLive(_1);
-          _1 = bar::<T>() -> [return: bb1, unwind continue];
+          _1 = bar::<T>() -> [return: bb1, unwind unreachable];
       }
 
       bb1: {

diff --git a/tests/mir-opt/inline/inline_coroutine.main.Inline.panic-unwind.diff b/tests/mir-opt/inline/inline_coroutine.main.Inline.panic-unwind.diff
@@ -31,7 +31,7 @@
           StorageLive(_2);
           StorageLive(_3);
           StorageLive(_4);
--         _4 = g() -> [return: bb1, unwind continue];
+-         _4 = g() -> [return: bb1, unwind unreachable];
 +         _4 = {coroutine@$DIR/inline_coroutine.rs:20:5: 20:8 (#0)};
 +         _3 = &mut _4;
 +         _2 = Pin::<&mut {coroutine@$DIR/inline_coroutine.rs:20:5: 20:8}> { __pointer: copy _3 };

diff --git a/tests/mir-opt/inline/inline_options.main.Inline.after.panic-unwind.mir b/tests/mir-opt/inline/inline_options.main.Inline.after.panic-unwind.mir
@@ -21,7 +21,7 @@ fn main() -> () {
         StorageLive(_3);
         StorageLive(_4);
         StorageLive(_5);
-        _3 = g() -> [return: bb3, unwind continue];
+        _3 = g() -> [return: bb3, unwind unreachable];
     }
 
     bb2: {
@@ -34,10 +34,10 @@ fn main() -> () {
     }
 
     bb3: {
-        _4 = g() -> [return: bb4, unwind continue];
+        _4 = g() -> [return: bb4, unwind unreachable];
     }
 
     bb4: {
-        _5 = g() -> [return: bb2, unwind continue];
+        _5 = g() -> [return: bb2, unwind unreachable];
     }
 }
diff --git a/tests/mir-opt/inline/inline_trait_method.test.Inline.after.panic-unwind.mir b/tests/mir-opt/inline/inline_trait_method.test.Inline.after.panic-unwind.mir
@@ -8,7 +8,7 @@ fn test(_1: &dyn X) -> u32 {
     bb0: {
         StorageLive(_2);
         _2 = copy _1;
-        _0 = <dyn X as X>::y(move _2) -> [return: bb1, unwind continue];
+        _0 = <dyn X as X>::y(move _2) -> [return: bb1, unwind unreachable];
     }
 
     bb1: {

diff --git a/tests/mir-opt/inline/inline_trait_method_2.test2.Inline.after.panic-unwind.mir b/tests/mir-opt/inline/inline_trait_method_2.test2.Inline.after.panic-unwind.mir
@@ -15,7 +15,7 @@ fn test2(_1: &dyn X) -> bool {
         _3 = copy _1;
         _2 = move _3;
         StorageDead(_3);
-        _0 = <dyn X as X>::y(move _2) -> [return: bb1, unwind continue];
+        _0 = <dyn X as X>::y(move _2) -> [return: bb1, unwind unreachable];
     }
 
     bb1: {

diff --git a/tests/mir-opt/inline/issue_78442.bar.Inline.panic-unwind.diff b/tests/mir-opt/inline/issue_78442.bar.Inline.panic-unwind.diff
@@ -15,7 +15,7 @@
           StorageLive(_2);
           StorageLive(_3);
           StorageLive(_4);
--         _4 = hide_foo() -> [return: bb1, unwind: bb4];
+-         _4 = hide_foo() -> [return: bb1, unwind unreachable];
 -     }
 - 
 -     bb1: {

diff --git a/tests/mir-opt/inline/rustc_no_mir_inline.caller.Inline.panic-unwind.diff b/tests/mir-opt/inline/rustc_no_mir_inline.caller.Inline.panic-unwind.diff
@@ -7,7 +7,7 @@
 
       bb0: {
           StorageLive(_1);
-          _1 = callee() -> [return: bb1, unwind continue];
+          _1 = callee() -> [return: bb1, unwind unreachable];
       }
 
       bb1: {

diff --git a/tests/mir-opt/inline/rustc_no_mir_inline.caller.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/inline/rustc_no_mir_inline.caller.PreCodegen.after.panic-unwind.mir
@@ -5,7 +5,7 @@ fn caller() -> () {
     let _1: ();
 
     bb0: {
-        _1 = callee() -> [return: bb1, unwind continue];
+        _1 = callee() -> [return: bb1, unwind unreachable];
     }
 
     bb1: {

diff --git a/tests/mir-opt/inline/unsized_argument.caller.Inline.diff b/tests/mir-opt/inline/unsized_argument.caller.Inline.diff
@@ -13,7 +13,7 @@
           StorageLive(_3);
           _3 = move _1;
           _4 = copy (((_3.0: std::ptr::Unique<[i32]>).0: std::ptr::NonNull<[i32]>).0: *const [i32]);
-          _2 = callee(move (*_4)) -> [return: bb1, unwind: bb3];
+          _2 = callee(move (*_4)) -> [return: bb1, unwind unreachable];
       }
 
       bb1: {

diff --git a/tests/mir-opt/pre-codegen/deref_nested_borrows.src.GVN.panic-unwind.diff b/tests/mir-opt/pre-codegen/deref_nested_borrows.src.GVN.panic-unwind.diff
@@ -20,7 +20,7 @@
 +         nop;
 +         _6 = copy (*_1);
           _2 = copy (*_6);
-          _3 = unknown() -> [return: bb1, unwind continue];
+          _3 = unknown() -> [return: bb1, unwind unreachable];
       }
 
       bb1: {
-Original file line number
+Diff line change
@@ Expand Up / @@ -10,7 +10,7 @@ @@
           bb0: {
               StorageLive(_1);
-              _1 = bar::<T>() -> [return: bb1, unwind continue];
+              _1 = bar::<T>() -> [return: bb1, unwind unreachable];
           }
           bb1: {
@@ Expand Down @@