Skip to content

Commit

Permalink
Auto merge of rust-lang#130455 - compiler-errors:inline-ordering, r=s…
Browse files Browse the repository at this point in the history
…aethlin

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
  • Loading branch information
bors committed Sep 17, 2024
2 parents 2e367d9 + 8f97231 commit 46b0f8b
Show file tree
Hide file tree
Showing 6 changed files with 4 additions and 72 deletions.
10 changes: 0 additions & 10 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())) {
Expand Down
21 changes: 1 addition & 20 deletions tests/mir-opt/inline/cycle.main.Inline.panic-abort.diff
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
}

29 changes: 1 addition & 28 deletions tests/mir-opt/inline/cycle.main.Inline.panic-unwind.diff
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

4 changes: 0 additions & 4 deletions tests/mir-opt/inline/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down

0 comments on commit 46b0f8b

Please sign in to comment.