From 10d83290616b50d1e6b7d0fce2c75555b7e3b4a8 Mon Sep 17 00:00:00 2001 From: dianqk Date: Thu, 16 Apr 2026 21:21:58 +0800 Subject: [PATCH] codegen: Copy to an alloca when the argument is neither by-val nor by-move for indirect pointer. --- compiler/rustc_codegen_ssa/src/mir/block.rs | 122 ++++++++++-------- tests/codegen-llvm/call-tmps-lifetime.rs | 13 +- tests/codegen-llvm/const-vector.rs | 3 +- .../indirect-bycopy-bymove-byval.rs | 16 +-- .../ui/moves/bycopy_untupled.noopt.run.stderr | 6 - tests/ui/moves/bycopy_untupled.opt.run.stderr | 1 - tests/ui/moves/bycopy_untupled.rs | 5 +- 7 files changed, 81 insertions(+), 85 deletions(-) delete mode 100644 tests/ui/moves/bycopy_untupled.noopt.run.stderr delete mode 100644 tests/ui/moves/bycopy_untupled.opt.run.stderr diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 1b96be12f71d..06c81662d601 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -1147,8 +1147,9 @@ fn codegen_call_terminator( // Special logic for tail calls with `PassMode::Indirect { on_stack: false, .. }` arguments. // - // Normally an indirect argument with `on_stack: false` would be passed as a pointer into - // the caller's stack frame. For tail calls, that would be unsound, because the caller's + // Normally an indirect argument that is allocated in the caller's stack frame + // would be passed as a pointer into the callee's stack frame. + // For tail calls, that would be unsound, because the caller's // stack frame is overwritten by the callee's stack frame. // // Therefore we store the argument for the callee in the corresponding caller's slot. @@ -1240,59 +1241,57 @@ fn codegen_call_terminator( } } - match kind { - CallKind::Normal => { - // The callee needs to own the argument memory if we pass it - // by-ref, so make a local copy of non-immediate constants. - if let &mir::Operand::Copy(_) | &mir::Operand::Constant(_) = &arg.node - && let Ref(PlaceValue { llextra: None, .. }) = op.val - { - let tmp = PlaceRef::alloca(bx, op.layout); - bx.lifetime_start(tmp.val.llval, tmp.layout.size); - op.store_with_annotation(bx, tmp); - op.val = Ref(tmp.val); - lifetime_ends_after_call.push((tmp.val.llval, tmp.layout.size)); + let by_move = if let PassMode::Indirect { on_stack: false, .. } = fn_abi.args[i].mode + && kind == CallKind::Tail + { + // Special logic for tail calls with `PassMode::Indirect { on_stack: false, .. }` arguments. + // + // Normally an indirect argument that is allocated in the caller's stack frame + // would be passed as a pointer into the callee's stack frame. + // For tail calls, that would be unsound, because the caller's + // stack frame is overwritten by the callee's stack frame. + // + // To handle the case, we introduce `tail_call_temporaries` to copy arguments into + // 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. + let Some(tmp) = tail_call_temporaries[i].take() else { + span_bug!(fn_span, "missing temporary for indirect tail call argument #{i}") + }; + + let local = self.mir.args_iter().nth(i).unwrap(); + + match &self.locals[local] { + LocalRef::Place(arg) => { + bx.typed_place_copy(arg.val, tmp.val, fn_abi.args[i].layout); + op.val = Ref(arg.val); } - } - CallKind::Tail => { - if let PassMode::Indirect { on_stack: false, .. } = fn_abi.args[i].mode { - let Some(tmp) = tail_call_temporaries[i].take() else { - span_bug!( - fn_span, - "missing temporary for indirect tail call argument #{i}" - ) + LocalRef::Operand(arg) => { + let Ref(place_value) = arg.val else { + bug!("only `Ref` should use `PassMode::Indirect`"); }; - - let local = self.mir.args_iter().nth(i).unwrap(); - - match &self.locals[local] { - LocalRef::Place(arg) => { - bx.typed_place_copy(arg.val, tmp.val, fn_abi.args[i].layout); - op.val = Ref(arg.val); - } - LocalRef::Operand(arg) => { - let Ref(place_value) = arg.val else { - bug!("only `Ref` should use `PassMode::Indirect`"); - }; - bx.typed_place_copy(place_value, tmp.val, fn_abi.args[i].layout); - op.val = arg.val; - } - LocalRef::UnsizedPlace(_) => { - span_bug!(fn_span, "unsized types are not supported") - } - LocalRef::PendingOperand => { - span_bug!(fn_span, "argument local should not be pending") - } - }; - - bx.lifetime_end(tmp.val.llval, tmp.layout.size); + bx.typed_place_copy(place_value, tmp.val, fn_abi.args[i].layout); + op.val = arg.val; } - } - } + LocalRef::UnsizedPlace(_) => { + span_bug!(fn_span, "unsized types are not supported") + } + LocalRef::PendingOperand => { + span_bug!(fn_span, "argument local should not be pending") + } + }; + + bx.lifetime_end(tmp.val.llval, tmp.layout.size); + true + } else { + matches!(arg.node, mir::Operand::Move(_)) + }; self.codegen_argument( bx, op, + by_move, &mut llargs, &fn_abi.args[i], &mut lifetime_ends_after_call, @@ -1331,6 +1330,7 @@ fn codegen_call_terminator( self.codegen_argument( bx, location, + /* by_move */ false, &mut llargs, last_arg, &mut lifetime_ends_after_call, @@ -1649,6 +1649,7 @@ fn codegen_argument( &mut self, bx: &mut Bx, op: OperandRef<'tcx, Bx::Value>, + by_move: bool, llargs: &mut Vec, arg: &ArgAbi<'tcx, Ty<'tcx>>, lifetime_ends_after_call: &mut Vec<(Bx::Value, Size)>, @@ -1703,18 +1704,19 @@ fn codegen_argument( _ => (op.immediate_or_packed_pair(bx), arg.layout.align.abi, false), }, Ref(op_place_val) => match arg.mode { - PassMode::Indirect { attrs, .. } => { + PassMode::Indirect { attrs, on_stack, .. } => { + // For `foo(packed.large_field)`, and types with <4 byte alignment on x86, + // alignment requirements may be higher than the type's alignment, so copy + // to a higher-aligned alloca. let required_align = match attrs.pointee_align { Some(pointee_align) => cmp::max(pointee_align, arg.layout.align.abi), None => arg.layout.align.abi, }; - if op_place_val.align < required_align { - // For `foo(packed.large_field)`, and types with <4 byte alignment on x86, - // alignment requirements may be higher than the type's alignment, so copy - // to a higher-aligned alloca. + // Copy to an alloca when the argument is neither by-val nor by-move. + if op_place_val.align < required_align || (!on_stack && !by_move) { let scratch = PlaceValue::alloca(bx, arg.layout.size, required_align); bx.lifetime_start(scratch.llval, arg.layout.size); - bx.typed_place_copy(scratch, op_place_val, op.layout); + op.store_with_annotation(bx, scratch.with_type(arg.layout)); lifetime_ends_after_call.push((scratch.llval, arg.layout.size)); (scratch.llval, scratch.align, true) } else { @@ -1800,6 +1802,7 @@ fn codegen_arguments_untupled( lifetime_ends_after_call: &mut Vec<(Bx::Value, Size)>, ) -> usize { let tuple = self.codegen_operand(bx, operand); + let by_move = matches!(operand, mir::Operand::Move(_)); // Handle both by-ref and immediate tuples. if let Ref(place_val) = tuple.val { @@ -1810,13 +1813,20 @@ fn codegen_arguments_untupled( for i in 0..tuple.layout.fields.count() { let field_ptr = tuple_ptr.project_field(bx, i); let field = bx.load_operand(field_ptr); - self.codegen_argument(bx, field, llargs, &args[i], lifetime_ends_after_call); + self.codegen_argument( + bx, + field, + by_move, + llargs, + &args[i], + lifetime_ends_after_call, + ); } } else { // If the tuple is immediate, the elements are as well. for i in 0..tuple.layout.fields.count() { let op = tuple.extract_field(self, bx, i); - self.codegen_argument(bx, op, llargs, &args[i], lifetime_ends_after_call); + self.codegen_argument(bx, op, by_move, llargs, &args[i], lifetime_ends_after_call); } } tuple.layout.fields.count() diff --git a/tests/codegen-llvm/call-tmps-lifetime.rs b/tests/codegen-llvm/call-tmps-lifetime.rs index 14c3f0c1e131..0e62cfc1e604 100644 --- a/tests/codegen-llvm/call-tmps-lifetime.rs +++ b/tests/codegen-llvm/call-tmps-lifetime.rs @@ -11,19 +11,12 @@ use minicore::*; // Const operand. Regression test for #98156. +// Temporary allocas are not required when passing as byval arguments. // // CHECK-LABEL: define void @const_indirect( // CHECK-NEXT: start: -// CHECK-NEXT: [[B:%.*]] = alloca -// CHECK-NEXT: [[A:%.*]] = alloca -// CHECK-NEXT: call void @llvm.lifetime.start.p0({{(i64 4096, )?}}ptr [[A]]) -// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[A]], ptr align 4 {{.*}}, i32 4096, i1 false) -// CHECK-NEXT: call void %h(ptr {{.*}} [[A]]) -// CHECK-NEXT: call void @llvm.lifetime.end.p0({{(i64 4096, )?}}ptr [[A]]) -// CHECK-NEXT: call void @llvm.lifetime.start.p0({{(i64 4096, )?}}ptr [[B]]) -// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[B]], ptr align 4 {{.*}}, i32 4096, i1 false) -// CHECK-NEXT: call void %h(ptr {{.*}} [[B]]) -// CHECK-NEXT: call void @llvm.lifetime.end.p0({{(i64 4096, )?}}ptr [[B]]) +// CHECK-NEXT: call void %h(ptr {{.*}}byval([4096 x i8]){{.*}} [[C:@anon.*]]) +// CHECK-NEXT: call void %h(ptr {{.*}}byval([4096 x i8]){{.*}} [[C]]) #[no_mangle] pub fn const_indirect(h: extern "C" fn([u32; 1024])) { const C: [u32; 1024] = [0; 1024]; diff --git a/tests/codegen-llvm/const-vector.rs b/tests/codegen-llvm/const-vector.rs index f43074923411..25d25d3e8ff3 100644 --- a/tests/codegen-llvm/const-vector.rs +++ b/tests/codegen-llvm/const-vector.rs @@ -74,7 +74,8 @@ pub fn do_call() { // CHECK: call void @test_simd(<4 x i32> test_simd(const { Simd::([2, 4, 6, 8]) }); - // CHECK: call void @test_simd_unaligned(%"minisimd::PackedSimd" %1 + // CHECK: [[UNALIGNED_ARG:%.*]] = load %"minisimd::PackedSimd", ptr @anon{{.*}} + // CHECK-NEXT: call void @test_simd_unaligned(%"minisimd::PackedSimd" [[UNALIGNED_ARG]] test_simd_unaligned(const { Simd::([2, 4, 6]) }); } } diff --git a/tests/codegen-llvm/indirect-bycopy-bymove-byval.rs b/tests/codegen-llvm/indirect-bycopy-bymove-byval.rs index 7908fa468287..a72aee6f424b 100644 --- a/tests/codegen-llvm/indirect-bycopy-bymove-byval.rs +++ b/tests/codegen-llvm/indirect-bycopy-bymove-byval.rs @@ -46,15 +46,17 @@ fn opaque(mut thing: Thing) { opaque(VALUE); } -// FIXME: closure#0 and closure#1 are missing memcpy. +// The argument of the second call is a by-move argument. // CHECK-LABEL: @untupled +// CHECK: call void @llvm.memcpy{{.*}}(ptr{{.*}} [[untupled_V1:%.*]], ptr{{.*}} %value // CHECK: call indirect_bycopy_bymove_byval::untupled::{closure#0} -// CHECK-NEXT: call void @{{.*}}(ptr {{.*}}, ptr{{.*}} %value) +// CHECK-NEXT: call void @{{.*}}(ptr {{.*}}, ptr{{.*}} [[untupled_V1]]) // CHECK: call indirect_bycopy_bymove_byval::untupled::{closure#1} // CHECK-NEXT: call void @{{.*}}(ptr {{.*}}, ptr{{.*}} %value) +// CHECK: call void @llvm.memcpy{{.*}}(ptr{{.*}} [[untupled_V3:%.*]], ptr{{.*}} @anon{{.*}} // CHECK: call indirect_bycopy_bymove_byval::untupled::{closure#2} -// CHECK-NEXT: call void @{{.*}}(ptr {{.*}}, ptr{{.*}} @anon{{.*}}) +// CHECK-NEXT: call void @{{.*}}(ptr {{.*}}, ptr{{.*}} [[untupled_V3]]) #[unsafe(no_mangle)] pub fn untupled() { let value = (Thing(0, 0, 0),); @@ -76,14 +78,12 @@ pub fn untupled() { .call(VALUE); } -// FIXME: all memcpy calls are redundant for byval. +// All memcpy calls are redundant for byval. // CHECK-LABEL: @byval -// CHECK: call void @llvm.memcpy{{.*}}(ptr{{.*}} [[byval_V1:%.*]], ptr{{.*}} %value, -// CHECK: call void @opaque_byval(ptr{{.*}} byval([24 x i8]){{.*}} [[byval_V1]]) // CHECK: call void @opaque_byval(ptr{{.*}} byval([24 x i8]){{.*}} %value) -// CHECK: call void @llvm.memcpy{{.*}}(ptr{{.*}} [[byval_V3:%.*]], ptr{{.*}} @anon{{.*}}, -// CHECK: call void @opaque_byval(ptr{{.*}} byval([24 x i8]){{.*}} [[byval_V3]]) +// CHECK: call void @opaque_byval(ptr{{.*}} byval([24 x i8]){{.*}} %value) +// CHECK: call void @opaque_byval(ptr{{.*}} byval([24 x i8]){{.*}} @anon{{.*}}) #[unsafe(no_mangle)] pub fn byval() { #[repr(C)] diff --git a/tests/ui/moves/bycopy_untupled.noopt.run.stderr b/tests/ui/moves/bycopy_untupled.noopt.run.stderr deleted file mode 100644 index 9eaded6c0165..000000000000 --- a/tests/ui/moves/bycopy_untupled.noopt.run.stderr +++ /dev/null @@ -1,6 +0,0 @@ - -thread 'main' ($TID) panicked at $DIR/bycopy_untupled.rs:25:5: -assertion `left == right` failed - left: 1 - right: 0 -note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/tests/ui/moves/bycopy_untupled.opt.run.stderr b/tests/ui/moves/bycopy_untupled.opt.run.stderr deleted file mode 100644 index 915e3ecb5436..000000000000 --- a/tests/ui/moves/bycopy_untupled.opt.run.stderr +++ /dev/null @@ -1 +0,0 @@ -free(): double free detected in tcache 2 diff --git a/tests/ui/moves/bycopy_untupled.rs b/tests/ui/moves/bycopy_untupled.rs index 9eca69e50379..c46c13bf7793 100644 --- a/tests/ui/moves/bycopy_untupled.rs +++ b/tests/ui/moves/bycopy_untupled.rs @@ -1,7 +1,6 @@ -//@[noopt] run-fail -//@[opt] run-crash +//! Regression test for issue . +//@ run-pass //@ revisions: noopt opt -//@ check-run-results //@[noopt] compile-flags: -C opt-level=0 //@[opt] compile-flags: -C opt-level=3