Auto merge of #147361 - saethlin:fallible-cycle-detection, r=oli-obk

Make inliner cycle detection a fallible process

The query `mir_callgraph_cyclic` is supposed to find _all_ callees that _may_ lead to a recursive call back to the given `LocalDefId`. But that query was built using a function which recurses through the call graph and tries to locally handle hitting the recursion limit during the walk. That is wrong. If the recursion limit is encountered, the set may be incomplete and thus useless. If we hit the recursion limit the only correct thing to do is bail.

Some benchmarks improve because for some functions we will bail out of the call graph walk faster. Some benchmarks regress because we do less inlining, but that is quite rare with the default recursion depth.

Originally I thought this might be a fix for #131960, but it turns out that it is _actually_ a fix for #146998.

Author: bors

compiler/rustc_middle/src/query/mod.rs

@@ -1312,8 +1312,9 @@ rustc_queries! {
         return_result_from_ensure_ok
     }
 
-    /// Return the set of (transitive) callees that may result in a recursive call to `key`.
-    query mir_callgraph_cyclic(key: LocalDefId) -> &'tcx UnordSet<LocalDefId> {
+    /// Return the set of (transitive) callees that may result in a recursive call to `key`,
+    /// if we were able to walk all callees.
+    query mir_callgraph_cyclic(key: LocalDefId) -> &'tcx Option<UnordSet<LocalDefId>> {
         fatal_cycle
         arena_cache
         desc { |tcx|

compiler/rustc_mir_transform/src/inline.rs

@@ -775,8 +775,11 @@ fn check_mir_is_available<'tcx, I: Inliner<'tcx>>(
     {
         // If we know for sure that the function we're calling will itself try to
         // call us, then we avoid inlining that function.
-        if inliner.tcx().mir_callgraph_cyclic(caller_def_id.expect_local()).contains(&callee_def_id)
-        {
+        let Some(cyclic_callees) = inliner.tcx().mir_callgraph_cyclic(caller_def_id.expect_local())
+        else {
+            return Err("call graph cycle detection bailed due to recursion limit");
+        };
+        if cyclic_callees.contains(&callee_def_id) {
             debug!("query cycle avoidance");
             return Err("caller might be reachable from callee");
         }

compiler/rustc_mir_transform/src/inline/cycle.rs

@@ -68,7 +68,7 @@ fn process<'tcx>(
     involved: &mut FxHashSet<LocalDefId>,
     recursion_limiter: &mut FxHashMap<DefId, usize>,
     recursion_limit: Limit,
-) -> bool {
+) -> Option<bool> {
     trace!(%caller);
     let mut reaches_root = false;
 
@@ -127,10 +127,9 @@ fn process<'tcx>(
                         recursion_limiter,
                         recursion_limit,
                     )
-                })
+                })?
             } else {
-                // Pessimistically assume that there could be recursion.
-                true
+                return None;
             };
             seen.insert(callee, callee_reaches_root);
             callee_reaches_root
@@ -144,14 +143,14 @@ fn process<'tcx>(
         }
     }
 
-    reaches_root
+    Some(reaches_root)
 }
 
 #[instrument(level = "debug", skip(tcx), ret)]
 pub(crate) fn mir_callgraph_cyclic<'tcx>(
     tcx: TyCtxt<'tcx>,
     root: LocalDefId,
-) -> UnordSet<LocalDefId> {
+) -> Option<UnordSet<LocalDefId>> {
     assert!(
         !tcx.is_constructor(root.to_def_id()),
         "you should not call `mir_callgraph_reachable` on enum/struct constructor functions"
@@ -164,16 +163,16 @@ pub(crate) fn mir_callgraph_cyclic<'tcx>(
     // limit, we will hit the limit first and give up on looking for inlining. And in any case,
     // the default recursion limits are quite generous for us. If we need to recurse 64 times
     // into the call graph, we're probably not going to find any useful MIR inlining.
-    let recursion_limit = tcx.recursion_limit() / 2;
+    let recursion_limit = tcx.recursion_limit() / 8;
     let mut involved = FxHashSet::default();
     let typing_env = ty::TypingEnv::post_analysis(tcx, root);
     let root_instance =
         ty::Instance::new_raw(root.to_def_id(), ty::GenericArgs::identity_for_item(tcx, root));
     if !should_recurse(tcx, root_instance) {
         trace!("cannot walk, skipping");
-        return involved.into();
+        return Some(involved.into());
     }
-    process(
+    match process(
         tcx,
         typing_env,
         root_instance,
@@ -182,8 +181,10 @@ pub(crate) fn mir_callgraph_cyclic<'tcx>(
         &mut involved,
         &mut FxHashMap::default(),
         recursion_limit,
-    );
-    involved.into()
+    ) {
+        Some(_) => Some(involved.into()),
+        _ => None,
+    }
 }
 
 pub(crate) fn mir_inliner_callees<'tcx>(

tests/mir-opt/inline/auxiliary/wrapper.rs

@@ -0,0 +1,10 @@
+//@ no-prefer-dynamic
+//@ compile-flags: -O
+
+pub trait Compare {
+    fn eq(self);
+}
+
+pub fn wrap<A: Compare>(a: A) {
+    Compare::eq(a);
+}

tests/mir-opt/inline/recursion_limit_prevents_cycle_discovery.rs

@@ -0,0 +1,54 @@
+//@ aux-build: wrapper.rs
+//@ compile-flags: -Zmir-opt-level=2 -Zinline-mir
+// skip-filecheck
+
+// This is a regression test for https://github.com/rust-lang/rust/issues/146998
+
+extern crate wrapper;
+use wrapper::{Compare, wrap};
+
+pub struct BundleInner;
+
+impl Compare for BundleInner {
+    fn eq(self) {
+        lots_of_calls();
+        wrap(Resource::ExtensionValue);
+    }
+}
+
+pub enum Resource {
+    Bundle,
+    ExtensionValue,
+}
+
+impl Compare for Resource {
+    fn eq(self) {
+        match self {
+            Self::Bundle => wrap(BundleInner),
+            Self::ExtensionValue => lots_of_calls(),
+        }
+    }
+}
+
+macro_rules! units {
+    ($($n: ident)*) => {
+        $(
+            struct $n;
+
+            impl Compare for $n {
+                fn eq(self) {  }
+            }
+
+            wrap($n);
+        )*
+    };
+}
+
+fn lots_of_calls() {
+    units! {
+        O1 O2 O3 O4 O5 O6 O7 O8 O9 O10 O11 O12 O13 O14 O15 O16 O17 O18 O19 O20
+        O21 O22 O23 O24 O25 O26 O27 O28 O29 O30 O31 O32 O33 O34 O35 O36 O37 O38 O39 O40
+        O41 O42 O43 O44 O45 O46 O47 O48 O49 O50 O51 O52 O53 O54 O55 O56 O57 O58 O59 O60
+        O61 O62 O63 O64 O65
+    }
+}