explicit tail calls: pass caller's argument slots as arguments for untuple indirect arguments by dianqk · Pull Request #158248 · rust-lang/rust

dianqk · 2026-06-22T12:56:34Z

tracking issue: #112788

Same as #151143, but for untuple indirect arguments.

r? WaffleLapkin

rustbot · 2026-06-22T12:56:41Z

WaffleLapkin is not on the review rotation at the moment.
They may take a while to respond.

dianqk · 2026-06-22T12:58:21Z

 // temporaries, then copy back to the caller's argument slots.
- // Finally, we pass the caller's argument slots as arguments.
- //
- // To do that, the argument must be MUST-by-move value.


MUST-by-move is removed in this PR.
FYI @nikic
View changes since the review

dianqk · 2026-06-22T13:03:47Z

- let local = self.mir.args_iter().nth(i).unwrap();
-
- match &self.locals[local] {


This is not a reliable way to get the LLVM value for the argument. We are allowed to introduce a copy alloca in this case:
rust/compiler/rustc_codegen_ssa/src/mir/mod.rs
Lines 539 to 541 in 9030e34
let llarg = bx.get_param(llarg_idx);
llarg_idx += 1;
LocalRef::Place(PlaceRef::new_sized(llarg, arg.layout))
.
cc @folkertdev
View changes since the review

WaffleLapkin · 2026-06-22T13:39:11Z

+ // We don't need `tail_call_temporaries` for the untuple arguments because they won't be
+ // modified, they are the last arguments.


I don't get what this comment is saying...
View changes since the review

The new comment should be clearer.

WaffleLapkin · 2026-06-23T10:30:38Z

+ // For untupled arguments, it is safe to store them directly into the caller's
+ // argument slots without temporaries, because the untupled arguments are
+ // always passed through a tuple alloca. The alloca serves as a temporary
+ // that does not overlap with any caller argument.
+ // No temporaries are needed for the caller's untupled arguments either,
+ // because they are used last.


I still don't quite get this. First of all, this should probably mention tail calls, as otherwise it doesn't make sense why anything here wouldn't be ok.
But second of all, I checked out your branch locally and I'm getting incorrect codegen (rustc t.rs --emit=llvm-ir -O):
#![feature(unboxed_closures)]#![feature(explicit_tail_calls)]#![crate_type = "lib"]typeTuple = (u64,u64,u64,u64);traitX{extern"rust-call"fnf(self,args:Tuple) -> u8;extern"rust-call"fng(self,args:Tuple) -> u8;}implXforTuple{extern"rust-call"fnf(self,args:Tuple) -> u8{ become args.g(self)}#[inline(never)]extern"rust-call"fng(self,args:Tuple) -> u8{ std::hint::black_box((self, args));0}}
; <(u64, u64, u64, u64) as t::X>::f; Function Attrs: nounwind nonlazybind uwtabledefine noundef i8@_RNvXCs146zwHEvxau_1tTyyyyENtB2_1X1f(ptr dead_on_return noalias nofree noundef align8 captures(none) dereferenceable(32) initializes((0, 32)) %self, i64 noundef %0, i64 noundef %1, i64 noundef %2, i64 noundef %3) unnamed_addr #0 { start: storei64%0, ptr%self, align8%.sroa.4.0.self.sroa_idx = getelementptrinboundsnuwi8, ptr%self, i648storei64%1, ptr%.sroa.4.0.self.sroa_idx, align8%.sroa.5.0.self.sroa_idx = getelementptrinboundsnuwi8, ptr%self, i6416storei64%2, ptr%.sroa.5.0.self.sroa_idx, align8%.sroa.6.0.self.sroa_idx = getelementptrinboundsnuwi8, ptr%self, i6424storei64%3, ptr%.sroa.6.0.self.sroa_idx, align8; call <(u64, u64, u64, u64) as t::X>::g%4 = musttail call noundef i8@_RNvXCs146zwHEvxau_1tTyyyyENtB2_1X1g(ptrnoalias nofree noundef nonnullalign8 captures(address) dereferenceable(32) %self, i64 noundef %0, i64 noundef %1, i64 noundef %2, i64 noundef %3) #4reti8%4 }
Granted, this code is weird and I couldn't come up with a more normal looking one, but I don't get the justification for why this is sound...
View changes since the review

I see. Thanks for the case.

WaffleLapkin · 2026-06-23T10:41:37Z

I could honestly see us just banning tail calling extern "rust-call" functions. There is really no reason to support that I think.

…direct arguments

dianqk · 2026-06-23T13:55:24Z

I could honestly see us just banning tail calling extern "rust-call" functions. There is really no reason to support that I think.

I don’t have a strong opinion on this decision. I think it’s up to you.

rust-log-analyzer · 2026-06-23T15:02:13Z

The job x86_64-gnu-tools failed! Check out the build log: (web)(plain enhanced)(plain)

Click to see the possible cause of the failure (guessed by this bot)

.........F.................................. (144/144)
======== tests/rustdoc-gui/sidebar-resize-window.goml ========
[ERROR] line 35: TimeoutError: Navigation timeout of 30000 ms exceeded: for command `reload:`
at <file:///checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/doc/test_docs/index.html>
[ERROR] line 36: ProtocolError: Emulation.setDeviceMetricsOverride timed out. Increase the 'protocolTimeout' setting in launch/connect calls for a higher timeout if needed.: for command `set-window-size: (1280, 600)`
at <file:///checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/doc/test_docs/index.html>
<= doc-ui tests done: 143 succeeded, 1 failed, 0 filtered out
Error: ()

WaffleLapkin · 2026-06-24T14:09:47Z

@folkertdev I'm tempted to say that we should disallow tail calling extern "rust-call" functions; do you have any opinions on this?

folkertdev · 2026-06-24T14:38:12Z

Not really, what are the implications of that?

WaffleLapkin · 2026-06-24T15:27:42Z

@folkertdev not much. The only reason to use rust-call is if you are implementing FnOnce for a closure(-like), and we already disallow tail calling closures.

folkertdev · 2026-06-24T16:40:53Z

Sounds like a good idea then 👍

…folkertdev disallow tail calling extern "rust-call" functions r? folkertdev Following discussion from rust-lang#158248. Fixesrust-lang#158017Closesrust-lang#158248

rust-bors · 2026-06-25T08:30:18Z

☔ The latest upstream changes (presumably #158342) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

…folkertdev disallow tail calling extern "rust-call" functions r? folkertdev Following discussion from rust-lang#158248. Fixesrust-lang#158017Closesrust-lang#158248

rustbot assigned WaffleLapkin Jun 22, 2026

rustbot added F-explicit_tail_calls `#![feature(explicit_tail_calls)]` S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 22, 2026

dianqk commented Jun 22, 2026
View reviewed changes

WaffleLapkin reviewed Jun 22, 2026
View reviewed changes

dianqk force-pushed the tail-untuple-indirect branch from 5493c9b to a61a2b4Compare June 23, 2026 06:35

WaffleLapkin reviewed Jun 23, 2026
View reviewed changes

explicit tail calls: pass caller's argument slots as arguments for in…
c48e49c
…direct arguments

dianqk force-pushed the tail-untuple-indirect branch from a61a2b4 to c48e49cCompare June 23, 2026 13:52

WaffleLapkin mentioned this pull request Jun 24, 2026
disallow tail calling extern "rust-call" functions #158374
Merged

rust-borsBot closed this in 005f989Jun 25, 2026

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 25, 2026

dianqk deleted the tail-untuple-indirect branch June 26, 2026 00:15

		let local = self.mir.args_iter().nth(i).unwrap();

		match &self.locals[local] {

	let llarg = bx.get_param(llarg_idx);
	llarg_idx += 1;
	LocalRef::Place(PlaceRef::new_sized(llarg, arg.layout))

		// We don't need `tail_call_temporaries` for the untuple arguments because they won't be
		// modified, they are the last arguments.

Uh oh!

Uh oh!

Conversation

dianqk commented Jun 22, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 22, 2026

Uh oh!

dianqkJun 22, 2026• edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dianqkJun 22, 2026• edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

WaffleLapkinJun 22, 2026• edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dianqkJun 23, 2026

Choose a reason for hiding this comment

Uh oh!

WaffleLapkinJun 23, 2026• edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dianqkJun 23, 2026

Choose a reason for hiding this comment

Uh oh!

WaffleLapkin commented Jun 23, 2026

Uh oh!

dianqk commented Jun 23, 2026

Uh oh!

rust-log-analyzer commented Jun 23, 2026

Uh oh!

WaffleLapkin commented Jun 24, 2026

Uh oh!

folkertdev commented Jun 24, 2026

Uh oh!

WaffleLapkin commented Jun 24, 2026

Uh oh!

folkertdev commented Jun 24, 2026

Uh oh!

rust-borsBot commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dianqk commented Jun 22, 2026•
edited
Loading

dianqkJun 22, 2026•
edited by rustbot
Loading

dianqkJun 22, 2026•
edited by rustbot
Loading

WaffleLapkinJun 22, 2026•
edited by rustbot
Loading

WaffleLapkinJun 23, 2026•
edited by rustbot
Loading