Auto merge of #130455 - compiler-errors:inline-ordering, r=saethlin

Remove semi-nondeterminism of `DefPathHash` ordering from inliner

Déjà vu or something because I kinda thought I had put this PR up before. I recall a discussion somewhere where I think it was `@saethlin` mentioning that this check was no longer needed since we have "proper" cycle detection. Putting that up as a PR now.

This may slighlty negatively affect inlining, since the cycle breaking here means that we still inlined some cycles when the def path hashes were ordered in certain ways, this leads to really bad nondeterminism that makes minimizing ICEs and putting up inliner bugfixes difficult.

r? `@cjgillot` or `@saethlin` or someone else idk

Author: bors

compiler/rustc_mir_transform/src/inline.rs

@@ -357,16 +357,6 @@ impl<'tcx> Inliner<'tcx> {
         }
 
         if callee_def_id.is_local() {
-            // Avoid a cycle here by only using `instance_mir` only if we have
-            // a lower `DefPathHash` than the callee. This ensures that the callee will
-            // not inline us. This trick even works with incremental compilation,
-            // since `DefPathHash` is stable.
-            if self.tcx.def_path_hash(caller_def_id).local_hash()
-                < self.tcx.def_path_hash(callee_def_id).local_hash()
-            {
-                return Ok(());
-            }
-
             // If we know for sure that the function we're calling will itself try to
             // call us, then we avoid inlining that function.
             if self.tcx.mir_callgraph_reachable((callee, caller_def_id.expect_local())) {

tests/mir-opt/inline/cycle.main.Inline.panic-abort.diff

@@ -4,35 +4,16 @@
   fn main() -> () {
       let mut _0: ();
       let _1: ();
-+     let mut _2: fn() {g};
-+     scope 1 (inlined f::<fn() {g}>) {
-+         debug g => _2;
-+         let mut _3: &fn() {g};
-+         let _4: ();
-+     }
 
       bb0: {
           StorageLive(_1);
--         _1 = f::<fn() {g}>(g) -> [return: bb1, unwind unreachable];
-+         StorageLive(_2);
-+         _2 = g;
-+         StorageLive(_4);
-+         StorageLive(_3);
-+         _3 = &_2;
-+         _4 = <fn() {g} as Fn<()>>::call(move _3, const ()) -> [return: bb2, unwind unreachable];
+          _1 = f::<fn() {g}>(g) -> [return: bb1, unwind unreachable];
       }
 
       bb1: {
-+         StorageDead(_4);
-+         StorageDead(_2);
           StorageDead(_1);
           _0 = const ();
           return;
-+     }
-+ 
-+     bb2: {
-+         StorageDead(_3);
-+         drop(_2) -> [return: bb1, unwind unreachable];
       }
   }
 

tests/mir-opt/inline/cycle.main.Inline.panic-unwind.diff

@@ -4,43 +4,16 @@
   fn main() -> () {
       let mut _0: ();
       let _1: ();
-+     let mut _2: fn() {g};
-+     scope 1 (inlined f::<fn() {g}>) {
-+         debug g => _2;
-+         let mut _3: &fn() {g};
-+         let _4: ();
-+     }
 
       bb0: {
           StorageLive(_1);
--         _1 = f::<fn() {g}>(g) -> [return: bb1, unwind continue];
-+         StorageLive(_2);
-+         _2 = g;
-+         StorageLive(_4);
-+         StorageLive(_3);
-+         _3 = &_2;
-+         _4 = <fn() {g} as Fn<()>>::call(move _3, const ()) -> [return: bb2, unwind: bb3];
+          _1 = f::<fn() {g}>(g) -> [return: bb1, unwind continue];
       }
 
       bb1: {
-+         StorageDead(_4);
-+         StorageDead(_2);
           StorageDead(_1);
           _0 = const ();
           return;
-+     }
-+ 
-+     bb2: {
-+         StorageDead(_3);
-+         drop(_2) -> [return: bb1, unwind continue];
-+     }
-+ 
-+     bb3 (cleanup): {
-+         drop(_2) -> [return: bb4, unwind terminate(cleanup)];
-+     }
-+ 
-+     bb4 (cleanup): {
-+         resume;
       }
   }
 

tests/mir-opt/inline/cycle.rs

@@ -19,9 +19,5 @@ fn g() {
 
 // EMIT_MIR cycle.main.Inline.diff
 fn main() {
-    // CHECK-LABEL: fn main(
-    // CHECK-NOT: inlined
-    // CHECK: (inlined f::<fn() {g}>)
-    // CHECK-NOT: inlined
     f(g);
 }

tests/mir-opt/inline/inline_cycle_generic.main.Inline.panic-abort.diff

@@ -7,18 +7,14 @@
 +     scope 1 (inlined <C as Call>::call) {
 +         scope 2 (inlined <B<A> as Call>::call) {
 +             scope 3 (inlined <A as Call>::call) {
-+                 scope 4 (inlined <B<C> as Call>::call) {
-+                     scope 5 (inlined <C as Call>::call) {
-+                     }
-+                 }
 +             }
 +         }
 +     }
 
       bb0: {
           StorageLive(_1);
 -         _1 = <C as Call>::call() -> [return: bb1, unwind unreachable];
-+         _1 = <B<A> as Call>::call() -> [return: bb1, unwind unreachable];
++         _1 = <B<C> as Call>::call() -> [return: bb1, unwind unreachable];
       }
 
       bb1: {

tests/mir-opt/inline/inline_cycle_generic.main.Inline.panic-unwind.diff

@@ -7,18 +7,14 @@
 +     scope 1 (inlined <C as Call>::call) {
 +         scope 2 (inlined <B<A> as Call>::call) {
 +             scope 3 (inlined <A as Call>::call) {
-+                 scope 4 (inlined <B<C> as Call>::call) {
-+                     scope 5 (inlined <C as Call>::call) {
-+                     }
-+                 }
 +             }
 +         }
 +     }
 
       bb0: {
           StorageLive(_1);
 -         _1 = <C as Call>::call() -> [return: bb1, unwind continue];
-+         _1 = <B<A> as Call>::call() -> [return: bb1, unwind continue];
++         _1 = <B<C> as Call>::call() -> [return: bb1, unwind continue];
       }
 
       bb1: {