Skip to content

Commit

Permalink
Auto merge of #110833 - compiler-errors:rustc-call-inliner-ice, r=cjg…
Browse files Browse the repository at this point in the history
…illot

Only unpack tupled args in inliner if we expect args to be unpacked

`"rust-call"` is a strange function abi. sometimes, it expects the arguments to be unpacked by the caller and passed as individual args (closure bodies), and sometimes it does not (user functions annotated with the `"rust-call"` abi).

make sure the mir inliner respects this difference when checking that arguments are compatible, and doesn't try to ICE when we call a `extern "rust-call"` function in a generic context.

fixes #110829
  • Loading branch information
bors committed Aug 4, 2023
2 parents 1fe3846 + e43649f commit 60fa393
Show file tree
Hide file tree
Showing 17 changed files with 255 additions and 159 deletions.
20 changes: 15 additions & 5 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,19 +223,29 @@ impl<'tcx> Inliner<'tcx> {
return Err("failed to normalize return type");
}
if callsite.fn_sig.abi() == Abi::RustCall {
let (arg_tuple, skipped_args) = match &args[..] {
[arg_tuple] => (arg_tuple, 0),
[_, arg_tuple] => (arg_tuple, 1),
// 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("do not inline 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),
_ => bug!("Expected `rust-call` to have 1 or 2 args"),
};

let self_arg_ty =
self_arg.map(|self_arg| self_arg.ty(&caller_body.local_decls, self.tcx));

let arg_tuple_ty = arg_tuple.ty(&caller_body.local_decls, self.tcx);
let ty::Tuple(arg_tuple_tys) = arg_tuple_ty.kind() else {
let ty::Tuple(arg_tuple_tys) = *arg_tuple_ty.kind() else {
bug!("Closure arguments are not passed as a tuple");
};

for (arg_ty, input) in
arg_tuple_tys.iter().zip(callee_body.args_iter().skip(skipped_args))
self_arg_ty.into_iter().chain(arg_tuple_tys).zip(callee_body.args_iter())
{
let input_type = callee_body.local_decls[input].ty;
if !util::is_subtype(self.tcx, self.param_env, input_type, arg_ty) {
Expand Down
8 changes: 1 addition & 7 deletions tests/mir-opt/inline/cycle.g.Inline.panic-abort.diff
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,10 @@
let mut _0: ();
let _1: ();
+ let mut _2: fn() {main};
+ let mut _5: ();
+ scope 1 (inlined f::<fn() {main}>) {
+ debug g => _2;
+ let mut _3: &fn() {main};
+ let _4: ();
+ scope 2 (inlined <fn() {main} as Fn<()>>::call - shim(fn() {main})) {
+ }
+ }

bb0: {
Expand All @@ -22,9 +19,7 @@
+ StorageLive(_4);
+ StorageLive(_3);
+ _3 = &_2;
+ StorageLive(_5);
+ _5 = const ();
+ _4 = move (*_3)() -> [return: bb2, unwind unreachable];
+ _4 = <fn() {main} as Fn<()>>::call(move _3, const ()) -> [return: bb2, unwind unreachable];
}

bb1: {
Expand All @@ -36,7 +31,6 @@
+ }
+
+ bb2: {
+ StorageDead(_5);
+ StorageDead(_3);
+ drop(_2) -> [return: bb1, unwind unreachable];
}
Expand Down
20 changes: 7 additions & 13 deletions tests/mir-opt/inline/cycle.g.Inline.panic-unwind.diff
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,10 @@
let mut _0: ();
let _1: ();
+ let mut _2: fn() {main};
+ let mut _5: ();
+ scope 1 (inlined f::<fn() {main}>) {
+ debug g => _2;
+ let mut _3: &fn() {main};
+ let _4: ();
+ scope 2 (inlined <fn() {main} as Fn<()>>::call - shim(fn() {main})) {
+ }
+ }

bb0: {
Expand All @@ -22,9 +19,7 @@
+ StorageLive(_4);
+ StorageLive(_3);
+ _3 = &_2;
+ StorageLive(_5);
+ _5 = const ();
+ _4 = move (*_3)() -> [return: bb4, unwind: bb2];
+ _4 = <fn() {main} as Fn<()>>::call(move _3, const ()) -> [return: bb2, unwind: bb3];
}

bb1: {
Expand All @@ -35,18 +30,17 @@
return;
+ }
+
+ bb2 (cleanup): {
+ drop(_2) -> [return: bb3, unwind terminate];
+ bb2: {
+ StorageDead(_3);
+ drop(_2) -> [return: bb1, unwind continue];
+ }
+
+ bb3 (cleanup): {
+ resume;
+ drop(_2) -> [return: bb4, unwind terminate];
+ }
+
+ bb4: {
+ StorageDead(_5);
+ StorageDead(_3);
+ drop(_2) -> [return: bb1, unwind continue];
+ bb4 (cleanup): {
+ resume;
}
}

18 changes: 1 addition & 17 deletions tests/mir-opt/inline/cycle.main.Inline.panic-abort.diff
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,10 @@
let mut _0: ();
let _1: ();
+ let mut _2: fn() {g};
+ let mut _5: ();
+ scope 1 (inlined f::<fn() {g}>) {
+ debug g => _2;
+ let mut _3: &fn() {g};
+ let _4: ();
+ scope 2 (inlined <fn() {g} as Fn<()>>::call - shim(fn() {g})) {
+ scope 3 (inlined g) {
+ scope 4 (inlined f::<fn() {main}>) {
+ debug g => main;
+ let _6: ();
+ scope 5 (inlined <fn() {main} as Fn<()>>::call - shim(fn() {main})) {
+ }
+ }
+ }
+ }
+ }

bb0: {
Expand All @@ -30,10 +19,7 @@
+ StorageLive(_4);
+ StorageLive(_3);
+ _3 = &_2;
+ StorageLive(_5);
+ _5 = const ();
+ StorageLive(_6);
+ _6 = main() -> [return: bb2, unwind unreachable];
+ _4 = <fn() {g} as Fn<()>>::call(move _3, const ()) -> [return: bb2, unwind unreachable];
}

bb1: {
Expand All @@ -45,8 +31,6 @@
+ }
+
+ bb2: {
+ StorageDead(_6);
+ StorageDead(_5);
+ StorageDead(_3);
+ drop(_2) -> [return: bb1, unwind unreachable];
}
Expand Down
30 changes: 7 additions & 23 deletions tests/mir-opt/inline/cycle.main.Inline.panic-unwind.diff
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,10 @@
let mut _0: ();
let _1: ();
+ let mut _2: fn() {g};
+ let mut _5: ();
+ scope 1 (inlined f::<fn() {g}>) {
+ debug g => _2;
+ let mut _3: &fn() {g};
+ let _4: ();
+ scope 2 (inlined <fn() {g} as Fn<()>>::call - shim(fn() {g})) {
+ scope 3 (inlined g) {
+ scope 4 (inlined f::<fn() {main}>) {
+ debug g => main;
+ let _6: ();
+ scope 5 (inlined <fn() {main} as Fn<()>>::call - shim(fn() {main})) {
+ }
+ }
+ }
+ }
+ }

bb0: {
Expand All @@ -30,10 +19,7 @@
+ StorageLive(_4);
+ StorageLive(_3);
+ _3 = &_2;
+ StorageLive(_5);
+ _5 = const ();
+ StorageLive(_6);
+ _6 = main() -> [return: bb4, unwind: bb2];
+ _4 = <fn() {g} as Fn<()>>::call(move _3, const ()) -> [return: bb2, unwind: bb3];
}

bb1: {
Expand All @@ -44,19 +30,17 @@
return;
+ }
+
+ bb2 (cleanup): {
+ drop(_2) -> [return: bb3, unwind terminate];
+ bb2: {
+ StorageDead(_3);
+ drop(_2) -> [return: bb1, unwind continue];
+ }
+
+ bb3 (cleanup): {
+ resume;
+ drop(_2) -> [return: bb4, unwind terminate];
+ }
+
+ bb4: {
+ StorageDead(_6);
+ StorageDead(_5);
+ StorageDead(_3);
+ drop(_2) -> [return: bb1, unwind continue];
+ bb4 (cleanup): {
+ resume;
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
- // MIR for `call` before Inline
+ // MIR for `call` after Inline

fn call(_1: Box<dyn FnMut<I, Output = ()>>, _2: I) -> () {
debug mock => _1;
debug input => _2;
let mut _0: ();
let mut _3: &mut std::boxed::Box<dyn std::ops::FnMut<I, Output = ()>>;
let mut _4: I;

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];
}

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

bb2: {
return;
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
- // MIR for `call` before Inline
+ // MIR for `call` after Inline

fn call(_1: Box<dyn FnMut<I, Output = ()>>, _2: I) -> () {
debug mock => _1;
debug input => _2;
let mut _0: ();
let mut _3: &mut std::boxed::Box<dyn std::ops::FnMut<I, Output = ()>>;
let mut _4: I;

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];
}

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

bb2: {
return;
}

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

bb4 (cleanup): {
resume;
}
}

11 changes: 11 additions & 0 deletions tests/mir-opt/inline/dont_ice_on_generic_rust_call.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
// compile-flags: -Zmir-enable-passes=+Inline --crate-type=lib

#![feature(fn_traits, tuple_trait, unboxed_closures)]

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) {
mock.call_mut(input)
}
32 changes: 32 additions & 0 deletions tests/mir-opt/inline/inline_box_fn.call.Inline.panic-abort.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
- // MIR for `call` before Inline
+ // MIR for `call` after Inline

fn call(_1: Box<dyn Fn(i32)>) -> () {
debug x => _1;
let mut _0: ();
let _2: ();
let mut _3: &std::boxed::Box<dyn std::ops::Fn(i32)>;
let mut _4: (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];
}

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

bb2: {
return;
}
}

40 changes: 40 additions & 0 deletions tests/mir-opt/inline/inline_box_fn.call.Inline.panic-unwind.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
- // MIR for `call` before Inline
+ // MIR for `call` after Inline

fn call(_1: Box<dyn Fn(i32)>) -> () {
debug x => _1;
let mut _0: ();
let _2: ();
let mut _3: &std::boxed::Box<dyn std::ops::Fn(i32)>;
let mut _4: (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];
}

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

bb2: {
return;
}

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

bb4 (cleanup): {
resume;
}
}

8 changes: 8 additions & 0 deletions tests/mir-opt/inline/inline_box_fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
// unit-test: Inline
// compile-flags: --crate-type=lib

// EMIT_MIR inline_box_fn.call.Inline.diff
fn call(x: Box<dyn Fn(i32)>) {
x(1);
}
Loading

0 comments on commit 60fa393

Please sign in to comment.