codegen: Copy to an alloca when the argument is neither by-val nor by-move for indirect pointer. by dianqk · Pull Request #155343 · rust-lang/rust

dianqk · 2026-04-15T14:42:16Z

When a value is passed via an indirect pointer, the value needs to be copied to a new alloca. For x86_64-unknown-linux-gnu, Thing is the case:

#[derive(Clone, Copy)]
struct Thing(usize, usize, usize);

pub fn foo() {
    let thing = Thing(0, 0, 0);
    bar(thing);
    assert_eq!(thing.0, 0);
}

#[inline(never)]
#[unsafe(no_mangle)]
pub fn bar(mut thing: Thing) {
    thing.0 = 1;
}

Before passing the thing to the bar function, the thing needs to be copied to an alloca that is passed to bar.

%0 = alloca [24 x i8], align 8
call void @llvm.memcpy.p0.p0.i64(ptr align 8 %0, ptr align 8 %thing, i64 24, i1 false)
call void @bar(ptr %0)

This patch applies the rule to the untupled arguments as well.

#![feature(fn_traits)]

#[derive(Clone, Copy)]
struct Thing(usize, usize, usize);

#[inline(never)]
#[unsafe(no_mangle)]
pub fn foo() {
    let thing = (Thing(0, 0, 0),);
    (|mut thing: Thing| {
        thing.0 = 1;
    }).call(thing);
    assert_eq!(thing.0.0, 0);
}

For this case, this patch changes from

; call example::foo::{closure#0}
call void @_RNCNvCs15qdZVLwHPA_7example3foo0B3_(ptr ..., ptr %thing)

to

%0 = alloca [24 x i8], align 8
call void @llvm.memcpy.p0.p0.i64(ptr align 8 %0, ptr align 8 %thing, i64 24, i1 false)
; call example::foo::{closure#0}
call void @_RNCNvCs15qdZVLwHPA_7example3foo0B3_(ptr ..., ptr %0)

However, the same rule cannot be applied to tail calls that would be unsound, because the caller's stack frame is overwritten by the callee's stack frame. Fortunately, #151143 has already handled the special case. We must not copy again.

No copy is needed for by-move arguments, because the argument is passed to the called "in-place".

No copy is also needed for by-val arguments, because the attribute implies that a hidden copy of the pointee is made between the caller and the callee.

NOTE: The patch has a trick for tail calls that we pass by-move. We can choose to copy an alloca even for by-move arguments, but tail calls require MUST-by-move.

dianqk · 2026-04-15T14:44:20Z

            self.codegen_argument(
                bx,
                op,
+                // Pass by-move for an explicit tail call, which has been handled above as well.


This feels a bit awkward; perhaps the tail call handling can be moved into codegen_argument as well.

View changes since the review

The issue has nothing to do with tail calls, does it...?

Yes. #151143 works well.

But this PR is changing something about tail calls...?

Maybe it's just the comment that is confusing me. Why is it okay to always to by-move for tail call arguments? Cc @folkertdev

Because #151143 has done a similar thing, we don't need to copy it again. IIUC, tail call is special; the same arguments on the caller must be passed to the callee. Extra copy is UB.

#151143 says something about

Therefore we store the argument for the callee in the corresponding caller's slot.

So I guess there is code elsewhere establishing an invariant that the argument here is the equivalent slot and we can pass a pointer to it. The comment should explain that and point at that code.

dianqk · 2026-04-15T14:48:36Z

        // CHECK: call void @test_simd(<4 x i32> <i32 2, i32 4, i32 6, i32 8>
        test_simd(const { Simd::<i32, 4>([2, 4, 6, 8]) });

-        // CHECK: call void @test_simd_unaligned(%"minisimd::PackedSimd<i32, 3>" %1


A redundant memcpy is skipped here.

View changes since the review

rustbot · 2026-04-16T13:24:10Z

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

RalfJung · 2026-04-20T16:44:09Z

            self.codegen_argument(
                bx,
                location,
+                false,


Suggested change

false,

/* by_move */ false,

View changes since the review

RalfJung · 2026-04-20T16:44:43Z

This makes sense to me at a high level, but I don't know the surrounding code so reviewing the exact details of the conditionals introduced here would take me more time than I can spare right now -- sorry. I hope we can find another reviewer.

maybe
r? @nikic

nikic · 2026-04-20T21:28:22Z

The PR description could use more words...

@bors try @rust-timer queue

codegen: Copy to an alloca when the argument is neither by-val nor by-move for indirect pointer.

rust-bors · 2026-04-20T23:35:46Z

☀️ Try build successful (CI)
Build commit: 6c71f51 (6c71f515200b0bbf6b3f6336c134d6d7ebe97f72, parent: c28e3037785af39226f5751294ed1c6cf4698e10)

rust-timer · 2026-04-21T00:30:12Z

Finished benchmarking commit (6c71f51): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (secondary -2.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.0%	[-2.0%, -2.0%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results (primary 2.5%, secondary 2.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.5%	[2.2%, 2.8%]	3
Regressions ❌ (secondary)	2.6%	[2.6%, 2.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.5%	[2.2%, 2.8%]	3

Binary size

Results (secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Bootstrap: 491.853s -> 492.621s (0.16%)
Artifact size: 394.39 MiB -> 394.38 MiB (-0.00%)

dianqk · 2026-04-21T13:37:48Z

The PR description could use more words...

Done. I also fixed the comment about tail calls.

nikic · 2026-04-21T13:44:49Z

+//! Arguments passed indirectly via a hidden pointer must be copied to an alloca,
+//! except for by-val or by-move.
+//@ compile-flags: -Cno-prepopulate-passes -Copt-level=3
+//@ only-x86_64-unknown-linux-gnu


Can this be made a minicore test instead?

View changes since the review

nikic · 2026-04-21T13:49:57Z

+                // 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.


What does "MUST-by-move" mean, as opposed to just by-move?

View changes since the review

I mean codegen allows copying for by-move, but not for MUST-by-move.

…-move for indirect pointer.

nikic · 2026-04-22T13:06:12Z

@bors r+

rust-bors · 2026-04-22T13:06:15Z

📌 Commit 10d8329 has been approved by nikic

It is now in the queue for this repository.

rust-bors · 2026-04-22T19:15:55Z

☀️ Test successful - CI
Approved by: nikic
Duration: 3h 28m 21s
Pushing f676c20 to main...

github-actions · 2026-04-22T19:18:57Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing cf1817b (parent) -> f676c20 (this PR)

Test differences

Show 14 test diffs

Stage 1

[codegen] tests/codegen-llvm/indirect-bycopy-bymove-byval.rs#i686-linux: [missing] -> pass (J2)
[codegen] tests/codegen-llvm/indirect-bycopy-bymove-byval.rs#i686-windows: [missing] -> pass (J2)
[codegen] tests/codegen-llvm/indirect-bycopy-bymove-byval.rs#x64-linux: [missing] -> pass (J2)
[ui] tests/ui/moves/bycopy_untupled.rs#noopt: [missing] -> pass (J3)
[ui] tests/ui/moves/bycopy_untupled.rs#opt: [missing] -> pass (J3)

Stage 2

[codegen] tests/codegen-llvm/indirect-bycopy-bymove-byval.rs#i686-linux: [missing] -> pass (J0)
[codegen] tests/codegen-llvm/indirect-bycopy-bymove-byval.rs#i686-windows: [missing] -> pass (J0)
[codegen] tests/codegen-llvm/indirect-bycopy-bymove-byval.rs#x64-linux: [missing] -> pass (J0)
[ui] tests/ui/moves/bycopy_untupled.rs#noopt: [missing] -> pass (J1)
[ui] tests/ui/moves/bycopy_untupled.rs#opt: [missing] -> pass (J1)

Additionally, 4 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard f676c20edd32321e9fa2781543d8970109707e30 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

dist-aarch64-linux: 2h 41m -> 1h 43m (-35.9%)
dist-x86_64-msvc-alt: 2h 8m -> 2h 49m (+31.7%)
test-various: 2h 10m -> 1h 29m (-31.6%)
dist-i686-mingw: 2h 10m -> 2h 47m (+28.2%)
i686-msvc-2: 2h 16m -> 1h 45m (-22.3%)
i686-msvc-1: 2h 22m -> 2h 54m (+22.2%)
x86_64-gnu-llvm-21-3: 1h 31m -> 1h 50m (+21.7%)
x86_64-mingw-2: 2h 39m -> 2h 6m (-21.0%)
pr-check-2: 33m 35s -> 40m 28s (+20.5%)
x86_64-gnu-llvm-21: 1h -> 1h 12m (+19.8%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2026-04-22T19:56:42Z

Finished benchmarking commit (f676c20): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

Results (primary 1.9%, secondary 2.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.9%	[1.7%, 2.2%]	2
Regressions ❌ (secondary)	2.0%	[2.0%, 2.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.9%	[1.7%, 2.2%]	2

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 491.458s -> 492.288s (0.17%)
Artifact size: 394.40 MiB -> 394.44 MiB (0.01%)

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 15, 2026

dianqk commented Apr 15, 2026

View reviewed changes

dianqk mentioned this pull request Apr 15, 2026

release-mode heap corruption with non-generic FnOnce(Vec<usize>) #155241

Closed

dianqk commented Apr 15, 2026

View reviewed changes

dianqk force-pushed the indirect-by-ref branch 2 times, most recently from 5123815 to 9ec6be7 Compare April 16, 2026 13:22

rustbot assigned RalfJung Apr 16, 2026

dianqk marked this pull request as ready for review April 16, 2026 13:24

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 16, 2026

dianqk changed the title ~~[DRAFT] codegen: Copy to an alloca when the argument is neither by-val nor by-move for indirect pointer.~~ codegen: Copy to an alloca when the argument is neither by-val nor by-move for indirect pointer. Apr 16, 2026

RalfJung reviewed Apr 20, 2026

View reviewed changes

rustbot assigned nikic and unassigned RalfJung Apr 20, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Apr 20, 2026

Auto merge of #155343 - dianqk:indirect-by-ref, r=<try>

6c71f51

codegen: Copy to an alloca when the argument is neither by-val nor by-move for indirect pointer.

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 20, 2026

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 21, 2026

dianqk force-pushed the indirect-by-ref branch from 9ec6be7 to ecd35c1 Compare April 21, 2026 13:36

nikic reviewed Apr 21, 2026

View reviewed changes

Add miscompiled test cases

e67e716

codegen: Copy to an alloca when the argument is neither by-val nor by…

10d8329

…-move for indirect pointer.

dianqk force-pushed the indirect-by-ref branch from ecd35c1 to 10d8329 Compare April 22, 2026 10:35

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 22, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 22, 2026

rust-bors Bot merged commit f676c20 into rust-lang:main Apr 22, 2026
12 checks passed

rustbot added this to the 1.97.0 milestone Apr 22, 2026

Uh oh!

Conversation

dianqk commented Apr 15, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dianqk Apr 15, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dianqk Apr 15, 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!

rustbot commented Apr 16, 2026

Uh oh!

RalfJung Apr 20, 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!

RalfJung commented Apr 20, 2026

Uh oh!

nikic commented Apr 20, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented Apr 20, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Apr 21, 2026

Overall result: no relevant changes - no action needed

Uh oh!

dianqk commented Apr 21, 2026

Uh oh!

nikic Apr 21, 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!

nikic Apr 21, 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!

Choose a reason for hiding this comment

Uh oh!

nikic commented Apr 22, 2026

Uh oh!

rust-bors Bot commented Apr 22, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Apr 22, 2026

Uh oh!

Uh oh!

github-actions Bot commented Apr 22, 2026

Test differences

Stage 1

Stage 2

Job duration changes

Uh oh!

rust-timer commented Apr 22, 2026

Overall result: no relevant changes - no action needed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dianqk commented Apr 15, 2026 •

edited by rustbot

Loading

dianqk Apr 15, 2026 •

edited by rustbot

Loading

dianqk Apr 15, 2026 •

edited by rustbot

Loading

RalfJung Apr 20, 2026 •

edited by rustbot

Loading

nikic Apr 21, 2026 •

edited by rustbot

Loading

nikic Apr 21, 2026 •

edited by rustbot

Loading