Skip to content

Commit 92e3dbb

Browse files
committed
Auto merge of #151143 - folkertdev:tail-call-indirect, r=<try>
explicit tail calls: support indirect arguments try-job: test-various
2 parents 0ee5907 + 106ccdb commit 92e3dbb

File tree

6 files changed

+311
-70
lines changed

6 files changed

+311
-70
lines changed

compiler/rustc_codegen_ssa/src/mir/block.rs

Lines changed: 105 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,19 +1146,51 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
11461146
(args, None)
11471147
};
11481148

1149+
// Special logic for tail calls with `PassMode::Indirect { on_stack: false, .. }` arguments.
1150+
//
1151+
// Normally an indirect argument with `on_stack: false` would be passed as a pointer into
1152+
// the caller's stack frame. For tail calls, that would be unsound, because the caller's
1153+
// stack frame is overwritten by the callee's stack frame.
1154+
//
1155+
// Therefore we store the argument for the callee in the corresponding caller's slot.
1156+
// Because guaranteed tail calls demand that the caller's signature matches the callee's,
1157+
// the corresponding slot has the correct type.
1158+
//
1159+
// To handle cases like the one below, the tail call arguments must first be copied to a
1160+
// temporary, and only then copied to the caller's argument slots.
1161+
//
1162+
// ```
1163+
// // A struct big enough that it is not passed via registers.
1164+
// pub struct Big([u64; 4]);
1165+
//
1166+
// fn swapper(a: Big, b: Big) -> (Big, Big) {
1167+
// become swapper_helper(b, a);
1168+
// }
1169+
// ```
1170+
let mut tail_call_temporaries = vec![];
1171+
if kind == CallKind::Tail {
1172+
tail_call_temporaries = vec![None; first_args.len()];
1173+
// Copy the arguments that use `PassMode::Indirect { on_stack: false , ..}`
1174+
// to temporary stack allocations. See the comment above.
1175+
for (i, arg) in first_args.iter().enumerate() {
1176+
if !matches!(fn_abi.args[i].mode, PassMode::Indirect { on_stack: false, .. }) {
1177+
continue;
1178+
}
1179+
1180+
let op = self.codegen_operand(bx, &arg.node);
1181+
let tmp = PlaceRef::alloca(bx, op.layout);
1182+
bx.lifetime_start(tmp.val.llval, tmp.layout.size);
1183+
op.store_with_annotation(bx, tmp);
1184+
1185+
tail_call_temporaries[i] = Some(tmp);
1186+
}
1187+
}
1188+
11491189
// When generating arguments we sometimes introduce temporary allocations with lifetime
11501190
// that extend for the duration of a call. Keep track of those allocations and their sizes
11511191
// to generate `lifetime_end` when the call returns.
11521192
let mut lifetime_ends_after_call: Vec<(Bx::Value, Size)> = Vec::new();
11531193
'make_args: for (i, arg) in first_args.iter().enumerate() {
1154-
if kind == CallKind::Tail && matches!(fn_abi.args[i].mode, PassMode::Indirect { .. }) {
1155-
// FIXME: https://github.com/rust-lang/rust/pull/144232#discussion_r2218543841
1156-
span_bug!(
1157-
fn_span,
1158-
"arguments using PassMode::Indirect are currently not supported for tail calls"
1159-
);
1160-
}
1161-
11621194
let mut op = self.codegen_operand(bx, &arg.node);
11631195

11641196
if let (0, Some(ty::InstanceKind::Virtual(_, idx))) = (i, instance.map(|i| i.def)) {
@@ -1209,18 +1241,72 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
12091241
}
12101242
}
12111243

1212-
// The callee needs to own the argument memory if we pass it
1213-
// by-ref, so make a local copy of non-immediate constants.
1214-
match (&arg.node, op.val) {
1215-
(&mir::Operand::Copy(_), Ref(PlaceValue { llextra: None, .. }))
1216-
| (&mir::Operand::Constant(_), Ref(PlaceValue { llextra: None, .. })) => {
1217-
let tmp = PlaceRef::alloca(bx, op.layout);
1218-
bx.lifetime_start(tmp.val.llval, tmp.layout.size);
1219-
op.store_with_annotation(bx, tmp);
1220-
op.val = Ref(tmp.val);
1221-
lifetime_ends_after_call.push((tmp.val.llval, tmp.layout.size));
1244+
match kind {
1245+
CallKind::Normal => {
1246+
// The callee needs to own the argument memory if we pass it
1247+
// by-ref, so make a local copy of non-immediate constants.
1248+
if let &mir::Operand::Copy(_) | &mir::Operand::Constant(_) = &arg.node
1249+
&& let Ref(PlaceValue { llextra: None, .. }) = op.val
1250+
{
1251+
let tmp = PlaceRef::alloca(bx, op.layout);
1252+
bx.lifetime_start(tmp.val.llval, tmp.layout.size);
1253+
op.store_with_annotation(bx, tmp);
1254+
op.val = Ref(tmp.val);
1255+
lifetime_ends_after_call.push((tmp.val.llval, tmp.layout.size));
1256+
}
1257+
}
1258+
CallKind::Tail => {
1259+
match fn_abi.args[i].mode {
1260+
PassMode::Indirect { on_stack: false, .. } => {
1261+
let Some(tmp) = tail_call_temporaries[i].take() else {
1262+
span_bug!(
1263+
fn_span,
1264+
"missing temporary for indirect tail call argument #{i}"
1265+
)
1266+
};
1267+
1268+
let local = self.mir.args_iter().nth(i).unwrap();
1269+
1270+
match &self.locals[local] {
1271+
LocalRef::Place(arg) => {
1272+
bx.typed_place_copy(arg.val, tmp.val, fn_abi.args[i].layout);
1273+
op.val = Ref(arg.val);
1274+
}
1275+
LocalRef::Operand(arg) => {
1276+
let Ref(place_value) = arg.val else {
1277+
bug!("only `Ref` should use `PassMode::Indirect`");
1278+
};
1279+
bx.typed_place_copy(
1280+
place_value,
1281+
tmp.val,
1282+
fn_abi.args[i].layout,
1283+
);
1284+
op.val = arg.val;
1285+
}
1286+
LocalRef::UnsizedPlace(_) => {
1287+
span_bug!(fn_span, "unsized types are not supported")
1288+
}
1289+
LocalRef::PendingOperand => {
1290+
span_bug!(fn_span, "argument local should not be pending")
1291+
}
1292+
};
1293+
1294+
bx.lifetime_end(tmp.val.llval, tmp.layout.size);
1295+
}
1296+
PassMode::Indirect { on_stack: true, .. } => {
1297+
// FIXME: some LLVM backends (notably x86) do not correctly pass byval
1298+
// arguments to tail calls (as of LLVM 21). See also:
1299+
//
1300+
// - https://github.com/rust-lang/rust/pull/144232#discussion_r2218543841
1301+
// - https://github.com/rust-lang/rust/issues/144855
1302+
span_bug!(
1303+
fn_span,
1304+
"arguments using PassMode::Indirect {{ on_stack: true, .. }} are currently not supported for tail calls"
1305+
)
1306+
}
1307+
_ => (),
1308+
}
12221309
}
1223-
_ => {}
12241310
}
12251311

12261312
self.codegen_argument(

compiler/rustc_mir_transform/src/deduce_param_attrs.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -121,17 +121,29 @@ impl<'tcx> Visitor<'tcx> for DeduceParamAttrs {
121121
// `f` passes. Note that function arguments are the only situation in which this problem can
122122
// arise: every other use of `move` in MIR doesn't actually write to the value it moves
123123
// from.
124-
if let TerminatorKind::Call { ref args, .. } = terminator.kind {
125-
for arg in args {
126-
if let Operand::Move(place) = arg.node
127-
&& !place.is_indirect_first_projection()
128-
&& let Some(i) = self.as_param(place.local)
129-
{
130-
self.usage[i] |= UsageSummary::MUTATE;
131-
self.usage[i] |= UsageSummary::CAPTURE;
124+
match terminator.kind {
125+
TerminatorKind::Call { ref args, .. } => {
126+
for arg in args {
127+
if let Operand::Move(place) = arg.node
128+
&& !place.is_indirect_first_projection()
129+
&& let Some(i) = self.as_param(place.local)
130+
{
131+
self.usage[i] |= UsageSummary::MUTATE;
132+
self.usage[i] |= UsageSummary::CAPTURE;
133+
}
132134
}
133135
}
134-
};
136+
137+
// Like a call, but more conservative because the backend may introduce writes to an
138+
// argument if the argument is passed as `PassMode::Indirect { on_stack: false, ... }`.
139+
TerminatorKind::TailCall { .. } => {
140+
for usage in self.usage.iter_mut() {
141+
*usage |= UsageSummary::MUTATE;
142+
*usage |= UsageSummary::CAPTURE;
143+
}
144+
}
145+
_ => {}
146+
}
135147

136148
self.super_terminator(terminator, location);
137149
}
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
//@ add-minicore
2+
//@ min-llvm-version: 22
3+
//@ assembly-output: emit-asm
4+
//@ needs-llvm-components: x86
5+
//@ compile-flags: --target=x86_64-unknown-linux-gnu
6+
//@ compile-flags: -Copt-level=3 -C llvm-args=-x86-asm-syntax=intel
7+
8+
#![feature(no_core, explicit_tail_calls)]
9+
#![expect(incomplete_features)]
10+
#![no_core]
11+
#![crate_type = "lib"]
12+
13+
// Test tail calls with `PassMode::Indirect { on_stack: false, .. }` arguments.
14+
//
15+
// Normally an indirect argument with `on_stack: false` would be passed as a pointer to the
16+
// caller's stack frame. For tail calls, that would be unsound, because the caller's stack
17+
// frame is overwritten by the callee's stack frame.
18+
//
19+
// The solution is to write the argument into the caller's argument place (stored somewhere further
20+
// up the stack), and forward that place.
21+
22+
extern crate minicore;
23+
use minicore::*;
24+
25+
#[repr(C)]
26+
struct S {
27+
x: u64,
28+
y: u64,
29+
z: u64,
30+
}
31+
32+
unsafe extern "C" {
33+
safe fn force_usage(_: u64, _: u64, _: u64) -> u64;
34+
}
35+
36+
// CHECK-LABEL: callee:
37+
// CHECK-NEXT: .cfi_startproc
38+
//
39+
// CHECK-NEXT: mov rax, qword ptr [rdi]
40+
// CHECK-NEXT: mov rsi, qword ptr [rdi + 8]
41+
// CHECK-NEXT: mov rdx, qword ptr [rdi + 16]
42+
// CHECK-NEXT: mov rdi, rax
43+
//
44+
// CHECK-NEXT: jmp qword ptr [rip + force_usage@GOTPCREL]
45+
#[inline(never)]
46+
#[unsafe(no_mangle)]
47+
fn callee(s: S) -> u64 {
48+
force_usage(s.x, s.y, s.z)
49+
}
50+
51+
// CHECK-LABEL: caller1:
52+
// CHECK-NEXT: .cfi_startproc
53+
//
54+
// Just forward the argument:
55+
//
56+
// CHECK-NEXT: jmp qword ptr [rip + callee@GOTPCREL]
57+
#[unsafe(no_mangle)]
58+
fn caller1(s: S) -> u64 {
59+
become callee(s);
60+
}
61+
62+
// CHECK-LABEL: caller2:
63+
// CHECK-NEXT: .cfi_startproc
64+
//
65+
// Construct the S value directly into the argument slot:
66+
//
67+
// CHECK-NEXT: mov qword ptr [rdi], 1
68+
// CHECK-NEXT: mov qword ptr [rdi + 8], 2
69+
// CHECK-NEXT: mov qword ptr [rdi + 16], 3
70+
//
71+
// CHECK-NEXT: jmp qword ptr [rip + callee@GOTPCREL]
72+
#[unsafe(no_mangle)]
73+
fn caller2(_: S) -> u64 {
74+
let s = S { x: 1, y: 2, z: 3 };
75+
become callee(s);
76+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
//@ add-minicore
2+
//@ revisions: win linux
3+
//@ min-llvm-version: 22
4+
//
5+
//@ compile-flags: -Copt-level=3
6+
//@[linux] compile-flags: --target x86_64-unknown-linux-gnu
7+
//@[linux] needs-llvm-components: x86
8+
//@[win] compile-flags: --target x86_64-pc-windows-msvc
9+
//@[win] needs-llvm-components: x86
10+
11+
#![crate_type = "lib"]
12+
#![feature(no_core, lang_items, explicit_tail_calls)]
13+
#![allow(incomplete_features)]
14+
#![no_core]
15+
16+
// Test passing of i128/u128, which is passed directly on x86, but indirectly on win64.
17+
18+
extern crate minicore;
19+
use minicore::*;
20+
21+
// linux: define noundef i128 @foo(i128 noundef %a, i128 noundef %b)
22+
// win: define <16 x i8> @foo(ptr {{.*}} %a, ptr {{.*}} %b)
23+
#[unsafe(no_mangle)]
24+
#[inline(never)]
25+
extern "C" fn foo(a: u128, b: u128) -> u128 {
26+
// linux: start:
27+
// linux-NEXT: musttail call noundef i128 @bar(i128 noundef %b, i128 noundef %a)
28+
//
29+
//
30+
// win: start:
31+
// win-NEXT: %0 = load i128, ptr %b
32+
// win-NEXT: %1 = load i128, ptr %a
33+
// win-NEXT: store i128 %0, ptr %a
34+
// win-NEXT: store i128 %1, ptr %b
35+
// win-NEXT: musttail call <16 x i8> @bar(ptr {{.*}} %a, ptr {{.*}} %b)
36+
become bar(b, a)
37+
}
38+
39+
unsafe extern "C" {
40+
safe fn bar(a: u128, b: u128) -> u128;
41+
}

tests/crashes/144293-indirect-ops-llvm.rs

Lines changed: 0 additions & 42 deletions
This file was deleted.

0 commit comments

Comments
 (0)