Skip to content

Commit

Permalink
Auto merge of rust-lang#137907 - compiler-errors:inline-fnonce, r=<try>
Browse files Browse the repository at this point in the history
Inline `FnOnce`/`FnMut`/`Fn` shims once again

This PR fixes the argument checking for `extern "rust-call"` ABI functions with a spread arg, which do no expect their arguments to be exploded from a tuple like closures do.

Secondly, it removes the hack that prevented them from being inlined. This results in more work done by the compiler, but it does end up allowing us to inline functions we didn't before.

Fixes rust-lang#137901
  • Loading branch information
bors committed Mar 3, 2025
2 parents e16a049 + d33946c commit 76aedcd
Show file tree
Hide file tree
Showing 23 changed files with 397 additions and 123 deletions.
36 changes: 16 additions & 20 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,14 +606,14 @@ fn try_inlining<'tcx, I: Inliner<'tcx>>(
ty::EarlyBinder::bind(callee_body.clone()),
) else {
debug!("failed to normalize callee body");
return Err("implementation limitation");
return Err("implementation limitation -- could not normalize callee body");
};

// Normally, this shouldn't be required, but trait normalization failure can create a
// validation ICE.
if !validate_types(tcx, inliner.typing_env(), &callee_body, &caller_body).is_empty() {
debug!("failed to validate callee body");
return Err("implementation limitation");
return Err("implementation limitation -- callee body failed validation");
}

// Check call signature compatibility.
Expand All @@ -622,17 +622,9 @@ fn try_inlining<'tcx, I: Inliner<'tcx>>(
let output_type = callee_body.return_ty();
if !util::sub_types(tcx, inliner.typing_env(), output_type, destination_ty) {
trace!(?output_type, ?destination_ty);
debug!("failed to normalize return type");
return Err("implementation limitation");
return Err("implementation limitation -- return type mismatch");
}
if callsite.fn_sig.abi() == ExternAbi::RustCall {
// FIXME: Don't inline user-written `extern "rust-call"` functions,
// since this is generally perf-negative on rustc, and we hope that
// LLVM will inline these functions instead.
if callee_body.spread_arg.is_some() {
return Err("user-written rust-call functions");
}

let (self_arg, arg_tuple) = match &args[..] {
[arg_tuple] => (None, arg_tuple),
[self_arg, arg_tuple] => (Some(self_arg), arg_tuple),
Expand All @@ -642,12 +634,17 @@ fn try_inlining<'tcx, I: Inliner<'tcx>>(
let self_arg_ty = self_arg.map(|self_arg| self_arg.node.ty(&caller_body.local_decls, tcx));

let arg_tuple_ty = arg_tuple.node.ty(&caller_body.local_decls, tcx);
let ty::Tuple(arg_tuple_tys) = *arg_tuple_ty.kind() else {
bug!("Closure arguments are not passed as a tuple");
let arg_tys = if callee_body.spread_arg.is_some() {
std::slice::from_ref(&arg_tuple_ty)
} else {
let ty::Tuple(arg_tuple_tys) = *arg_tuple_ty.kind() else {
bug!("Closure arguments are not passed as a tuple");
};
arg_tuple_tys.as_slice()
};

for (arg_ty, input) in
self_arg_ty.into_iter().chain(arg_tuple_tys).zip(callee_body.args_iter())
self_arg_ty.into_iter().chain(arg_tys.iter().copied()).zip(callee_body.args_iter())
{
let input_type = callee_body.local_decls[input].ty;
if !util::sub_types(tcx, inliner.typing_env(), input_type, arg_ty) {
Expand All @@ -663,7 +660,7 @@ fn try_inlining<'tcx, I: Inliner<'tcx>>(
if !util::sub_types(tcx, inliner.typing_env(), input_type, arg_ty) {
trace!(?arg_ty, ?input_type);
debug!("failed to normalize argument type");
return Err("implementation limitation");
return Err("implementation limitation -- arg mismatch");
}
}
}
Expand Down Expand Up @@ -693,13 +690,13 @@ fn check_mir_is_available<'tcx, I: Inliner<'tcx>>(
// won't cause cycles on this.
if !inliner.tcx().is_mir_available(callee_def_id) {
debug!("item MIR unavailable");
return Err("implementation limitation");
return Err("implementation limitation -- MIR unavailable");
}
}
// These have no own callable MIR.
InstanceKind::Intrinsic(_) | InstanceKind::Virtual(..) => {
debug!("instance without MIR (intrinsic / virtual)");
return Err("implementation limitation");
return Err("implementation limitation -- cannot inline intrinsic");
}

// FIXME(#127030): `ConstParamHasTy` has bad interactions with
Expand All @@ -709,7 +706,7 @@ fn check_mir_is_available<'tcx, I: Inliner<'tcx>>(
// substituted.
InstanceKind::DropGlue(_, Some(ty)) if ty.has_type_flags(TypeFlags::HAS_CT_PARAM) => {
debug!("still needs substitution");
return Err("implementation limitation");
return Err("implementation limitation -- HACK for dropping polymorphic type");
}

// This cannot result in an immediate cycle since the callee MIR is a shim, which does
Expand Down Expand Up @@ -1060,8 +1057,7 @@ fn make_call_args<'tcx, I: Inliner<'tcx>>(

closure_ref_arg.chain(tuple_tmp_args).collect()
} else {
// FIXME(edition_2024): switch back to a normal method call.
<_>::into_iter(args)
args.into_iter()
.map(|a| create_temp_if_necessary(inliner, a.node, callsite, caller_body, return_block))
.collect()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,42 @@
let mut _0: ();
let mut _3: &mut std::boxed::Box<dyn std::ops::FnMut<I, Output = ()>>;
let mut _4: I;
+ scope 1 (inlined <Box<dyn FnMut<I, Output = ()>> as FnMut<I>>::call_mut) {
+ let mut _5: &mut dyn std::ops::FnMut<I, Output = ()>;
+ let mut _6: std::boxed::Box<dyn std::ops::FnMut<I, Output = ()>>;
+ let mut _7: *const dyn std::ops::FnMut<I, Output = ()>;
+ }

bb0: {
StorageLive(_3);
_3 = &mut _1;
StorageLive(_4);
_4 = move _2;
_0 = <Box<dyn FnMut<I, Output = ()>> as FnMut<I>>::call_mut(move _3, move _4) -> [return: bb1, unwind unreachable];
- _0 = <Box<dyn FnMut<I, Output = ()>> as FnMut<I>>::call_mut(move _3, move _4) -> [return: bb1, unwind unreachable];
+ StorageLive(_6);
+ StorageLive(_7);
+ StorageLive(_5);
+ _6 = copy (*_3);
+ _7 = copy ((_6.0: std::ptr::Unique<dyn std::ops::FnMut<I, Output = ()>>).0: std::ptr::NonNull<dyn std::ops::FnMut<I, Output = ()>>) as *const dyn std::ops::FnMut<I, Output = ()> (Transmute);
+ _5 = &mut (*_7);
+ _0 = <dyn FnMut<I, Output = ()> as FnMut<I>>::call_mut(move _5, move _4) -> [return: bb2, unwind unreachable];
}

bb1: {
StorageDead(_4);
StorageDead(_3);
drop(_1) -> [return: bb2, unwind unreachable];
- StorageDead(_4);
- StorageDead(_3);
- drop(_1) -> [return: bb2, unwind unreachable];
+ return;
}

bb2: {
return;
- return;
+ StorageDead(_5);
+ StorageDead(_7);
+ StorageDead(_6);
+ StorageDead(_4);
+ StorageDead(_3);
+ drop(_1) -> [return: bb1, unwind unreachable];
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,54 @@
let mut _0: ();
let mut _3: &mut std::boxed::Box<dyn std::ops::FnMut<I, Output = ()>>;
let mut _4: I;
+ scope 1 (inlined <Box<dyn FnMut<I, Output = ()>> as FnMut<I>>::call_mut) {
+ let mut _5: &mut dyn std::ops::FnMut<I, Output = ()>;
+ let mut _6: std::boxed::Box<dyn std::ops::FnMut<I, Output = ()>>;
+ let mut _7: *const dyn std::ops::FnMut<I, Output = ()>;
+ }

bb0: {
StorageLive(_3);
_3 = &mut _1;
StorageLive(_4);
_4 = move _2;
_0 = <Box<dyn FnMut<I, Output = ()>> as FnMut<I>>::call_mut(move _3, move _4) -> [return: bb1, unwind: bb3];
- _0 = <Box<dyn FnMut<I, Output = ()>> as FnMut<I>>::call_mut(move _3, move _4) -> [return: bb1, unwind: bb3];
+ StorageLive(_6);
+ StorageLive(_7);
+ StorageLive(_5);
+ _6 = copy (*_3);
+ _7 = copy ((_6.0: std::ptr::Unique<dyn std::ops::FnMut<I, Output = ()>>).0: std::ptr::NonNull<dyn std::ops::FnMut<I, Output = ()>>) as *const dyn std::ops::FnMut<I, Output = ()> (Transmute);
+ _5 = &mut (*_7);
+ _0 = <dyn FnMut<I, Output = ()> as FnMut<I>>::call_mut(move _5, move _4) -> [return: bb4, unwind: bb2];
}

bb1: {
StorageDead(_4);
StorageDead(_3);
drop(_1) -> [return: bb2, unwind: bb4];
- StorageDead(_4);
- StorageDead(_3);
- drop(_1) -> [return: bb2, unwind: bb4];
+ return;
}

bb2: {
return;
- bb2: {
- return;
+ bb2 (cleanup): {
+ drop(_1) -> [return: bb3, unwind terminate(cleanup)];
}

bb3 (cleanup): {
drop(_1) -> [return: bb4, unwind terminate(cleanup)];
- drop(_1) -> [return: bb4, unwind terminate(cleanup)];
+ resume;
}

bb4 (cleanup): {
resume;
- bb4 (cleanup): {
- resume;
+ bb4: {
+ StorageDead(_5);
+ StorageDead(_7);
+ StorageDead(_6);
+ StorageDead(_4);
+ StorageDead(_3);
+ drop(_1) -> [return: bb1, unwind: bb3];
}
}

2 changes: 1 addition & 1 deletion tests/mir-opt/inline/dont_ice_on_generic_rust_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ use std::marker::Tuple;
// EMIT_MIR dont_ice_on_generic_rust_call.call.Inline.diff
pub fn call<I: Tuple>(mut mock: Box<dyn FnMut<I, Output = ()>>, input: I) {
// CHECK-LABEL: fn call(
// CHECK-NOT: inlined
// CHECK: (inlined <Box<dyn FnMut<I, Output = ()>> as FnMut<I>>::call_mut)
mock.call_mut(input)
}
32 changes: 26 additions & 6 deletions tests/mir-opt/inline/inline_box_fn.call.Inline.panic-abort.diff
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,46 @@
let _2: ();
let mut _3: &std::boxed::Box<dyn std::ops::Fn(i32)>;
let mut _4: (i32,);
+ scope 1 (inlined <Box<dyn Fn(i32)> as Fn<(i32,)>>::call) {
+ let mut _5: &dyn std::ops::Fn(i32);
+ let mut _6: std::boxed::Box<dyn std::ops::Fn(i32)>;
+ let mut _7: *const dyn std::ops::Fn(i32);
+ }

bb0: {
StorageLive(_2);
StorageLive(_3);
_3 = &_1;
StorageLive(_4);
_4 = (const 1_i32,);
_2 = <Box<dyn Fn(i32)> as Fn<(i32,)>>::call(move _3, move _4) -> [return: bb1, unwind unreachable];
- _2 = <Box<dyn Fn(i32)> as Fn<(i32,)>>::call(move _3, move _4) -> [return: bb1, unwind unreachable];
+ StorageLive(_6);
+ StorageLive(_7);
+ StorageLive(_5);
+ _6 = copy (*_3);
+ _7 = copy ((_6.0: std::ptr::Unique<dyn std::ops::Fn(i32)>).0: std::ptr::NonNull<dyn std::ops::Fn(i32)>) as *const dyn std::ops::Fn(i32) (Transmute);
+ _5 = &(*_7);
+ _2 = <dyn Fn(i32) as Fn<(i32,)>>::call(move _5, move _4) -> [return: bb2, unwind unreachable];
}

bb1: {
+ return;
+ }
+
+ bb2: {
+ StorageDead(_5);
+ StorageDead(_7);
+ StorageDead(_6);
StorageDead(_4);
StorageDead(_3);
StorageDead(_2);
_0 = const ();
drop(_1) -> [return: bb2, unwind unreachable];
}

bb2: {
return;
- drop(_1) -> [return: bb2, unwind unreachable];
- }
-
- bb2: {
- return;
+ drop(_1) -> [return: bb1, unwind unreachable];
}
}

47 changes: 36 additions & 11 deletions tests/mir-opt/inline/inline_box_fn.call.Inline.panic-unwind.diff
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,59 @@
let _2: ();
let mut _3: &std::boxed::Box<dyn std::ops::Fn(i32)>;
let mut _4: (i32,);
+ scope 1 (inlined <Box<dyn Fn(i32)> as Fn<(i32,)>>::call) {
+ let mut _5: &dyn std::ops::Fn(i32);
+ let mut _6: std::boxed::Box<dyn std::ops::Fn(i32)>;
+ let mut _7: *const dyn std::ops::Fn(i32);
+ }

bb0: {
StorageLive(_2);
StorageLive(_3);
_3 = &_1;
StorageLive(_4);
_4 = (const 1_i32,);
_2 = <Box<dyn Fn(i32)> as Fn<(i32,)>>::call(move _3, move _4) -> [return: bb1, unwind: bb3];
- _2 = <Box<dyn Fn(i32)> as Fn<(i32,)>>::call(move _3, move _4) -> [return: bb1, unwind: bb3];
+ StorageLive(_6);
+ StorageLive(_7);
+ StorageLive(_5);
+ _6 = copy (*_3);
+ _7 = copy ((_6.0: std::ptr::Unique<dyn std::ops::Fn(i32)>).0: std::ptr::NonNull<dyn std::ops::Fn(i32)>) as *const dyn std::ops::Fn(i32) (Transmute);
+ _5 = &(*_7);
+ _2 = <dyn Fn(i32) as Fn<(i32,)>>::call(move _5, move _4) -> [return: bb4, unwind: bb2];
}

bb1: {
StorageDead(_4);
StorageDead(_3);
StorageDead(_2);
_0 = const ();
drop(_1) -> [return: bb2, unwind: bb4];
- StorageDead(_4);
- StorageDead(_3);
- StorageDead(_2);
- _0 = const ();
- drop(_1) -> [return: bb2, unwind: bb4];
+ return;
}

bb2: {
return;
- bb2: {
- return;
+ bb2 (cleanup): {
+ drop(_1) -> [return: bb3, unwind terminate(cleanup)];
}

bb3 (cleanup): {
drop(_1) -> [return: bb4, unwind terminate(cleanup)];
- drop(_1) -> [return: bb4, unwind terminate(cleanup)];
+ resume;
}

bb4 (cleanup): {
resume;
- bb4 (cleanup): {
- resume;
+ bb4: {
+ StorageDead(_5);
+ StorageDead(_7);
+ StorageDead(_6);
+ StorageDead(_4);
+ StorageDead(_3);
+ StorageDead(_2);
+ _0 = const ();
+ drop(_1) -> [return: bb1, unwind: bb3];
}
}

2 changes: 1 addition & 1 deletion tests/mir-opt/inline/inline_box_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
// EMIT_MIR inline_box_fn.call.Inline.diff
fn call(x: Box<dyn Fn(i32)>) {
// CHECK-LABEL: fn call(
// CHECK-NOT: inlined
// CHECK: (inlined <Box<dyn Fn(i32)> as Fn<(i32,)>>::call)
x(1);
}
13 changes: 12 additions & 1 deletion tests/mir-opt/inline/inline_cycle.two.Inline.panic-abort.diff
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@
let mut _0: ();
let _1: ();
+ let mut _2: fn() {f};
+ let mut _4: ();
+ scope 1 (inlined call::<fn() {f}>) {
+ debug f => _2;
+ let _3: ();
+ scope 2 (inlined <fn() {f} as FnOnce<()>>::call_once - shim(fn() {f})) {
+ scope 3 (inlined f) {
+ let _5: ();
+ }
+ }
+ }

bb0: {
Expand All @@ -16,10 +22,15 @@
+ StorageLive(_2);
+ _2 = f;
+ StorageLive(_3);
+ _3 = <fn() {f} as FnOnce<()>>::call_once(move _2, const ()) -> [return: bb1, unwind unreachable];
+ StorageLive(_4);
+ _4 = const ();
+ StorageLive(_5);
+ _5 = call::<fn() {f}>(f) -> [return: bb1, unwind unreachable];
}

bb1: {
+ StorageDead(_5);
+ StorageDead(_4);
+ StorageDead(_3);
+ StorageDead(_2);
StorageDead(_1);
Expand Down
Loading

0 comments on commit 76aedcd

Please sign in to comment.