-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Describe lifetime of call argument temporaries passed indirectly #138489
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,6 @@ | ||||||||
use std::cmp; | ||||||||
|
||||||||
use rustc_abi::{BackendRepr, ExternAbi, HasDataLayout, Reg, WrappingRange}; | ||||||||
use rustc_abi::{BackendRepr, ExternAbi, HasDataLayout, Reg, Size, WrappingRange}; | ||||||||
use rustc_ast as ast; | ||||||||
use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece}; | ||||||||
use rustc_data_structures::packed::Pu128; | ||||||||
|
@@ -158,7 +158,7 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { | |||||||
llargs: &[Bx::Value], | ||||||||
destination: Option<(ReturnDest<'tcx, Bx::Value>, mir::BasicBlock)>, | ||||||||
mut unwind: mir::UnwindAction, | ||||||||
copied_constant_arguments: &[PlaceRef<'tcx, <Bx as BackendTypes>::Value>], | ||||||||
lifetime_ends_after_call: &[(Bx::Value, Size)], | ||||||||
instance: Option<Instance<'tcx>>, | ||||||||
mergeable_succ: bool, | ||||||||
) -> MergingSucc { | ||||||||
|
@@ -245,8 +245,8 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { | |||||||
if let Some((ret_dest, target)) = destination { | ||||||||
bx.switch_to_block(fx.llbb(target)); | ||||||||
fx.set_debug_loc(bx, self.terminator.source_info); | ||||||||
for tmp in copied_constant_arguments { | ||||||||
bx.lifetime_end(tmp.val.llval, tmp.layout.size); | ||||||||
for &(tmp, size) in lifetime_ends_after_call { | ||||||||
bx.lifetime_end(tmp, size); | ||||||||
} | ||||||||
fx.store_return(bx, ret_dest, &fn_abi.ret, invokeret); | ||||||||
} | ||||||||
|
@@ -259,8 +259,8 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { | |||||||
} | ||||||||
|
||||||||
if let Some((ret_dest, target)) = destination { | ||||||||
for tmp in copied_constant_arguments { | ||||||||
bx.lifetime_end(tmp.val.llval, tmp.layout.size); | ||||||||
for &(tmp, size) in lifetime_ends_after_call { | ||||||||
bx.lifetime_end(tmp, size); | ||||||||
} | ||||||||
fx.store_return(bx, ret_dest, &fn_abi.ret, llret); | ||||||||
self.funclet_br(fx, bx, target, mergeable_succ) | ||||||||
|
@@ -1049,7 +1049,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |||||||
(args, None) | ||||||||
}; | ||||||||
|
||||||||
let mut copied_constant_arguments = vec![]; | ||||||||
// Keeps track of temporary allocas whose liftime need to be ended after the call. | ||||||||
let mut lifetime_ends_after_call: Vec<(Bx::Value, Size)> = Vec::new(); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also comments should generally prefer to include "why": probably this?
Suggested change
Comment on lines
-1052
to
+1053
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also one thing we kinda lose here with this rename is that most of the entities in this list are arguments or owned by the effective arguments: that's why we accumulate them through |
||||||||
'make_args: for (i, arg) in first_args.iter().enumerate() { | ||||||||
let mut op = self.codegen_operand(bx, &arg.node); | ||||||||
|
||||||||
|
@@ -1137,19 +1138,26 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |||||||
bx.lifetime_start(tmp.val.llval, tmp.layout.size); | ||||||||
op.val.store(bx, tmp); | ||||||||
op.val = Ref(tmp.val); | ||||||||
copied_constant_arguments.push(tmp); | ||||||||
lifetime_ends_after_call.push((tmp.val.llval, tmp.layout.size)); | ||||||||
} | ||||||||
_ => {} | ||||||||
} | ||||||||
|
||||||||
self.codegen_argument(bx, op, &mut llargs, &fn_abi.args[i]); | ||||||||
self.codegen_argument( | ||||||||
bx, | ||||||||
op, | ||||||||
&mut llargs, | ||||||||
&fn_abi.args[i], | ||||||||
&mut lifetime_ends_after_call, | ||||||||
); | ||||||||
} | ||||||||
let num_untupled = untuple.map(|tup| { | ||||||||
self.codegen_arguments_untupled( | ||||||||
bx, | ||||||||
&tup.node, | ||||||||
&mut llargs, | ||||||||
&fn_abi.args[first_args.len()..], | ||||||||
&mut lifetime_ends_after_call, | ||||||||
) | ||||||||
}); | ||||||||
|
||||||||
|
@@ -1174,7 +1182,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |||||||
); | ||||||||
|
||||||||
let last_arg = fn_abi.args.last().unwrap(); | ||||||||
self.codegen_argument(bx, location, &mut llargs, last_arg); | ||||||||
self.codegen_argument( | ||||||||
bx, | ||||||||
location, | ||||||||
&mut llargs, | ||||||||
last_arg, | ||||||||
&mut lifetime_ends_after_call, | ||||||||
); | ||||||||
} | ||||||||
|
||||||||
let fn_ptr = match (instance, llfn) { | ||||||||
|
@@ -1190,7 +1204,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |||||||
&llargs, | ||||||||
destination, | ||||||||
unwind, | ||||||||
&copied_constant_arguments, | ||||||||
&lifetime_ends_after_call, | ||||||||
instance, | ||||||||
mergeable_succ, | ||||||||
) | ||||||||
|
@@ -1475,6 +1489,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |||||||
op: OperandRef<'tcx, Bx::Value>, | ||||||||
llargs: &mut Vec<Bx::Value>, | ||||||||
arg: &ArgAbi<'tcx, Ty<'tcx>>, | ||||||||
lifetime_ends_after_call: &mut Vec<(Bx::Value, Size)>, | ||||||||
) { | ||||||||
match arg.mode { | ||||||||
PassMode::Ignore => return, | ||||||||
|
@@ -1513,7 +1528,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |||||||
None => arg.layout.align.abi, | ||||||||
}; | ||||||||
let scratch = PlaceValue::alloca(bx, arg.layout.size, required_align); | ||||||||
bx.lifetime_start(scratch.llval, arg.layout.size); | ||||||||
op.val.store(bx, scratch.with_type(arg.layout)); | ||||||||
lifetime_ends_after_call.push((scratch.llval, arg.layout.size)); | ||||||||
(scratch.llval, scratch.align, true) | ||||||||
} | ||||||||
PassMode::Cast { .. } => { | ||||||||
|
@@ -1534,7 +1551,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |||||||
// alignment requirements may be higher than the type's alignment, so copy | ||||||||
// to a higher-aligned alloca. | ||||||||
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); | ||||||||
lifetime_ends_after_call.push((scratch.llval, arg.layout.size)); | ||||||||
(scratch.llval, scratch.align, true) | ||||||||
} else { | ||||||||
(op_place_val.llval, op_place_val.align, true) | ||||||||
|
@@ -1616,6 +1635,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |||||||
operand: &mir::Operand<'tcx>, | ||||||||
llargs: &mut Vec<Bx::Value>, | ||||||||
args: &[ArgAbi<'tcx, Ty<'tcx>>], | ||||||||
lifetime_ends_after_call: &mut Vec<(Bx::Value, Size)>, | ||||||||
) -> usize { | ||||||||
let tuple = self.codegen_operand(bx, operand); | ||||||||
|
||||||||
|
@@ -1628,13 +1648,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { | |||||||
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]); | ||||||||
self.codegen_argument(bx, field, 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]); | ||||||||
self.codegen_argument(bx, op, llargs, &args[i], lifetime_ends_after_call); | ||||||||
} | ||||||||
} | ||||||||
tuple.layout.fields.count() | ||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
// Test that temporary allocas used for call arguments have their lifetimes described by | ||
// intrinsics. | ||
// | ||
//@ add-core-stubs | ||
//@ compile-flags: -Copt-level=1 -Cno-prepopulate-passes --crate-type=lib --target i686-unknown-linux-gnu | ||
//@ needs-llvm-components: x86 | ||
#![feature(no_core)] | ||
#![no_std] | ||
#![no_core] | ||
extern crate minicore; | ||
use minicore::*; | ||
|
||
// Const operand. Regression test for #98156. | ||
// | ||
// 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]]) | ||
#[no_mangle] | ||
pub fn const_indirect(h: extern "C" fn([u32; 1024])) { | ||
const C: [u32; 1024] = [0; 1024]; | ||
h(C); | ||
h(C); | ||
} | ||
|
||
#[repr(C)] | ||
pub struct Str { | ||
pub ptr: *const u8, | ||
pub len: usize, | ||
} | ||
|
||
// Pair of immediates. Regression test for #132014. | ||
// | ||
// CHECK-LABEL: define void @immediate_indirect(ptr {{.*}}%s.0, i32 {{.*}}%s.1, ptr {{.*}}%g) | ||
// CHECK-NEXT: start: | ||
// CHECK-NEXT: [[A:%.*]] = alloca | ||
// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 8, ptr [[A]]) | ||
// CHECK-NEXT: store ptr %s.0, ptr [[A]] | ||
// CHECK-NEXT: [[B:%.]] = getelementptr inbounds i8, ptr [[A]], i32 4 | ||
// CHECK-NEXT: store i32 %s.1, ptr [[B]] | ||
// CHECK-NEXT: call void %g(ptr {{.*}} [[A]]) | ||
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 8, ptr [[A]]) | ||
#[no_mangle] | ||
pub fn immediate_indirect(s: Str, g: extern "C" fn(Str)) { | ||
g(s); | ||
} | ||
|
||
// Indirect argument with a higher alignment requirement than the type's. | ||
// | ||
// CHECK-LABEL: define void @align_indirect(ptr{{.*}} align 1{{.*}} %a, ptr{{.*}} %fun) | ||
// CHECK-NEXT: start: | ||
// CHECK-NEXT: [[A:%.*]] = alloca [1024 x i8], align 4 | ||
// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 1024, ptr [[A]]) | ||
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[A]], ptr align 1 %a, i32 1024, i1 false) | ||
// CHECK-NEXT: call void %fun(ptr {{.*}} [[A]]) | ||
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 1024, ptr [[A]]) | ||
#[no_mangle] | ||
pub fn align_indirect(a: [u8; 1024], fun: extern "C" fn([u8; 1024])) { | ||
fun(a); | ||
} |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, also slightly confusing wording to me, and often comments like this are written in the imperative, so perhaps this could be more like
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exact phrasing isn't too important tho' and I don't necessarily insist on the imperative, please feel free to revise as you see fit.