Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline FnOnce/FnMut/Fn shims once again #137907

Merged
merged 2 commits into from
Mar 5, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 16 additions & 20 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
@@ -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.
@@ -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),
@@ -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) {
@@ -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");
}
}
}
@@ -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");
Comment on lines 697 to +699
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we can inline intrinsics, but we don't want to.

if tcx.has_attr(def_id, sym::rustc_intrinsic) {
// Intrinsic fallback bodies are always cross-crate inlineable.
// To ensure that the MIR inliner doesn't cluelessly try to inline fallback
// bodies even when the backend would implement something better, we stop
// the MIR inliner from ever inlining an intrinsic.
return true;
}

}

// FIXME(#127030): `ConstParamHasTy` has bad interactions with
@@ -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
@@ -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()
}
Original file line number Diff line number Diff line change
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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: {
@@ -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);
Loading
Oops, something went wrong.
Loading
Oops, something went wrong.