Skip to content

Commit

Permalink
Rollup merge of rust-lang#64435 - eddyb:arguments-against-arg, r=rkruppe
Browse files Browse the repository at this point in the history
codegen: use "_N" (like for other locals) instead of "argN", for argument names.

Based on rust-lang#64408 (second commit is new), fixing something I mentioned in rust-lang#64408 (which turned to be an immediate blocker for unifying relevant codepaths).

Closes rust-lang#64408 (by containing it).

r? @rkruppe
  • Loading branch information
Centril authored Sep 13, 2019
2 parents 99e9fc1 + bdad2c5 commit 589e238
Show file tree
Hide file tree
Showing 18 changed files with 98 additions and 110 deletions.
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ impl ArgTypeExt<'ll, 'tcx> for ArgType<'tcx, Ty<'tcx>> {
// We instead thus allocate some scratch space...
let scratch_size = cast.size(bx);
let scratch_align = cast.align(bx);
let llscratch = bx.alloca(cast.llvm_type(bx), "abi_cast", scratch_align);
let llscratch = bx.alloca(cast.llvm_type(bx), scratch_align);
bx.lifetime_start(llscratch, scratch_size);

// ...where we first store the value...
Expand Down
23 changes: 5 additions & 18 deletions src/librustc_codegen_llvm/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,23 +387,17 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
)
}

fn alloca(&mut self, ty: &'ll Type, name: &str, align: Align) -> &'ll Value {
fn alloca(&mut self, ty: &'ll Type, align: Align) -> &'ll Value {
let mut bx = Builder::with_cx(self.cx);
bx.position_at_start(unsafe {
llvm::LLVMGetFirstBasicBlock(self.llfn())
});
bx.dynamic_alloca(ty, name, align)
bx.dynamic_alloca(ty, align)
}

fn dynamic_alloca(&mut self, ty: &'ll Type, name: &str, align: Align) -> &'ll Value {
fn dynamic_alloca(&mut self, ty: &'ll Type, align: Align) -> &'ll Value {
unsafe {
let alloca = if name.is_empty() {
llvm::LLVMBuildAlloca(self.llbuilder, ty, UNNAMED)
} else {
let name = SmallCStr::new(name);
llvm::LLVMBuildAlloca(self.llbuilder, ty,
name.as_ptr())
};
let alloca = llvm::LLVMBuildAlloca(self.llbuilder, ty, UNNAMED);
llvm::LLVMSetAlignment(alloca, align.bytes() as c_uint);
alloca
}
Expand All @@ -412,16 +406,9 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
fn array_alloca(&mut self,
ty: &'ll Type,
len: &'ll Value,
name: &str,
align: Align) -> &'ll Value {
unsafe {
let alloca = if name.is_empty() {
llvm::LLVMBuildArrayAlloca(self.llbuilder, ty, len, UNNAMED)
} else {
let name = SmallCStr::new(name);
llvm::LLVMBuildArrayAlloca(self.llbuilder, ty, len,
name.as_ptr())
};
let alloca = llvm::LLVMBuildArrayAlloca(self.llbuilder, ty, len, UNNAMED);
llvm::LLVMSetAlignment(alloca, align.bytes() as c_uint);
alloca
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ fn codegen_msvc_try(
// More information can be found in libstd's seh.rs implementation.
let i64p = bx.type_ptr_to(bx.type_i64());
let ptr_align = bx.tcx().data_layout.pointer_align.abi;
let slot = bx.alloca(i64p, "slot", ptr_align);
let slot = bx.alloca(i64p, ptr_align);
bx.invoke(func, &[data], normal.llbb(), catchswitch.llbb(), None);

normal.ret(bx.const_i32(0));
Expand Down
18 changes: 9 additions & 9 deletions src/librustc_codegen_ssa/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let llslot = match op.val {
Immediate(_) | Pair(..) => {
let scratch =
PlaceRef::alloca(&mut bx, self.fn_ty.ret.layout, "ret");
PlaceRef::alloca(&mut bx, self.fn_ty.ret.layout);
op.val.store(&mut bx, scratch);
scratch.llval
}
Expand Down Expand Up @@ -767,7 +767,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
match (arg, op.val) {
(&mir::Operand::Copy(_), Ref(_, None, _)) |
(&mir::Operand::Constant(_), Ref(_, None, _)) => {
let tmp = PlaceRef::alloca(&mut bx, op.layout, "const");
let tmp = PlaceRef::alloca(&mut bx, op.layout);
op.val.store(&mut bx, tmp);
op.val = Ref(tmp.llval, None, tmp.align);
}
Expand Down Expand Up @@ -925,7 +925,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
Immediate(_) | Pair(..) => {
match arg.mode {
PassMode::Indirect(..) | PassMode::Cast(_) => {
let scratch = PlaceRef::alloca(bx, arg.layout, "arg");
let scratch = PlaceRef::alloca(bx, arg.layout);
op.val.store(bx, scratch);
(scratch.llval, scratch.align, true)
}
Expand All @@ -940,7 +940,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// think that ATM (Rust 1.16) we only pass temporaries, but we shouldn't
// have scary latent bugs around.

let scratch = PlaceRef::alloca(bx, arg.layout, "arg");
let scratch = PlaceRef::alloca(bx, arg.layout);
base::memcpy_ty(bx, scratch.llval, scratch.align, llval, align,
op.layout, MemFlags::empty());
(scratch.llval, scratch.align, true)
Expand Down Expand Up @@ -1017,7 +1017,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
cx.tcx().mk_mut_ptr(cx.tcx().types.u8),
cx.tcx().types.i32
]));
let slot = PlaceRef::alloca(bx, layout, "personalityslot");
let slot = PlaceRef::alloca(bx, layout);
self.personality_slot = Some(slot);
slot
}
Expand Down Expand Up @@ -1116,15 +1116,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
return if fn_ret.is_indirect() {
// Odd, but possible, case, we have an operand temporary,
// but the calling convention has an indirect return.
let tmp = PlaceRef::alloca(bx, fn_ret.layout, "tmp_ret");
let tmp = PlaceRef::alloca(bx, fn_ret.layout);
tmp.storage_live(bx);
llargs.push(tmp.llval);
ReturnDest::IndirectOperand(tmp, index)
} else if is_intrinsic {
// Currently, intrinsics always need a location to store
// the result, so we create a temporary `alloca` for the
// result.
let tmp = PlaceRef::alloca(bx, fn_ret.layout, "tmp_ret");
let tmp = PlaceRef::alloca(bx, fn_ret.layout);
tmp.storage_live(bx);
ReturnDest::IndirectOperand(tmp, index)
} else {
Expand Down Expand Up @@ -1174,7 +1174,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
LocalRef::Operand(None) => {
let dst_layout = bx.layout_of(self.monomorphized_place_ty(&dst.as_ref()));
assert!(!dst_layout.ty.has_erasable_regions());
let place = PlaceRef::alloca(bx, dst_layout, "transmute_temp");
let place = PlaceRef::alloca(bx, dst_layout);
place.storage_live(bx);
self.codegen_transmute_into(bx, src, place);
let op = bx.load_operand(place);
Expand Down Expand Up @@ -1227,7 +1227,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
DirectOperand(index) => {
// If there is a cast, we have to store and reload.
let op = if let PassMode::Cast(_) = ret_ty.mode {
let tmp = PlaceRef::alloca(bx, ret_ty.layout, "tmp_ret");
let tmp = PlaceRef::alloca(bx, ret_ty.layout);
tmp.storage_live(bx);
bx.store_arg_ty(&ret_ty, llval, tmp);
let op = bx.load_operand(tmp);
Expand Down
29 changes: 17 additions & 12 deletions src/librustc_codegen_ssa/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,13 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
debug!("alloc: {:?} ({}) -> place", local, name);
if layout.is_unsized() {
let indirect_place =
PlaceRef::alloca_unsized_indirect(&mut bx, layout, &name.as_str());
PlaceRef::alloca_unsized_indirect(&mut bx, layout);
bx.set_var_name(indirect_place.llval, name);
// FIXME: add an appropriate debuginfo
LocalRef::UnsizedPlace(indirect_place)
} else {
let place = PlaceRef::alloca(&mut bx, layout, &name.as_str());
let place = PlaceRef::alloca(&mut bx, layout);
bx.set_var_name(place.llval, name);
if dbg {
let (scope, span) = fx.debug_loc(mir::SourceInfo {
span: decl.source_info.span,
Expand All @@ -293,14 +295,13 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
} else if memory_locals.contains(local) {
debug!("alloc: {:?} -> place", local);
if layout.is_unsized() {
let indirect_place = PlaceRef::alloca_unsized_indirect(
&mut bx,
layout,
&format!("{:?}", local),
);
let indirect_place = PlaceRef::alloca_unsized_indirect(&mut bx, layout);
bx.set_var_name(indirect_place.llval, format_args!("{:?}", local));
LocalRef::UnsizedPlace(indirect_place)
} else {
LocalRef::Place(PlaceRef::alloca(&mut bx, layout, &format!("{:?}", local)))
let place = PlaceRef::alloca(&mut bx, layout);
bx.set_var_name(place.llval, format_args!("{:?}", local));
LocalRef::Place(place)
}
} else {
// If this is an immediate local, we do not create an
Expand Down Expand Up @@ -452,10 +453,11 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
mir.args_iter().enumerate().map(|(arg_index, local)| {
let arg_decl = &mir.local_decls[local];

// FIXME(eddyb) don't allocate a `String` unless it gets used.
let name = if let Some(name) = arg_decl.name {
name.as_str().to_string()
} else {
format!("arg{}", arg_index)
format!("{:?}", local)
};

if Some(local) == mir.spread_arg {
Expand All @@ -470,7 +472,8 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
_ => bug!("spread argument isn't a tuple?!")
};

let place = PlaceRef::alloca(bx, bx.layout_of(arg_ty), &name);
let place = PlaceRef::alloca(bx, bx.layout_of(arg_ty));
bx.set_var_name(place.llval, name);
for i in 0..tupled_arg_tys.len() {
let arg = &fx.fn_ty.args[idx];
idx += 1;
Expand Down Expand Up @@ -558,11 +561,13 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
llarg_idx += 1;
let indirect_operand = OperandValue::Pair(llarg, llextra);

let tmp = PlaceRef::alloca_unsized_indirect(bx, arg.layout, &name);
let tmp = PlaceRef::alloca_unsized_indirect(bx, arg.layout);
bx.set_var_name(tmp.llval, name);
indirect_operand.store(bx, tmp);
tmp
} else {
let tmp = PlaceRef::alloca(bx, arg.layout, &name);
let tmp = PlaceRef::alloca(bx, arg.layout);
bx.set_var_name(tmp.llval, name);
if fx.fn_ty.c_variadic && last_arg_idx.map(|idx| arg_index == idx).unwrap_or(false) {
let va_list_did = match tcx.lang_items().va_list() {
Some(did) => did,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_ssa/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue<V> {

// Allocate an appropriate region on the stack, and copy the value into it
let (llsize, _) = glue::size_and_align_of_dst(bx, unsized_ty, Some(llextra));
let lldst = bx.array_alloca(bx.cx().type_i8(), llsize, "unsized_tmp", max_align);
let lldst = bx.array_alloca(bx.cx().type_i8(), llsize, max_align);
bx.memcpy(lldst, max_align, llptr, min_align, llsize, flags);

// Store the allocated region and the extra to the indirect place.
Expand Down
8 changes: 2 additions & 6 deletions src/librustc_codegen_ssa/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,25 +71,21 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
pub fn alloca<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
bx: &mut Bx,
layout: TyLayout<'tcx>,
name: &str
) -> Self {
debug!("alloca({:?}: {:?})", name, layout);
assert!(!layout.is_unsized(), "tried to statically allocate unsized place");
let tmp = bx.alloca(bx.cx().backend_type(layout), name, layout.align.abi);
let tmp = bx.alloca(bx.cx().backend_type(layout), layout.align.abi);
Self::new_sized(tmp, layout)
}

/// Returns a place for an indirect reference to an unsized place.
pub fn alloca_unsized_indirect<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
bx: &mut Bx,
layout: TyLayout<'tcx>,
name: &str,
) -> Self {
debug!("alloca_unsized_indirect({:?}: {:?})", name, layout);
assert!(layout.is_unsized(), "tried to allocate indirect place for sized values");
let ptr_ty = bx.cx().tcx().mk_mut_ptr(layout.ty);
let ptr_layout = bx.cx().layout_of(ptr_ty);
Self::alloca(bx, ptr_layout, name)
Self::alloca(bx, ptr_layout)
}

pub fn len<Cx: ConstMethods<'tcx, Value = V>>(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_ssa/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// index into the struct, and this case isn't
// important enough for it.
debug!("codegen_rvalue: creating ugly alloca");
let scratch = PlaceRef::alloca(&mut bx, operand.layout, "__unsize_temp");
let scratch = PlaceRef::alloca(&mut bx, operand.layout);
scratch.storage_live(&mut bx);
operand.val.store(&mut bx, scratch);
base::coerce_unsized_into(&mut bx, scratch, dest);
Expand Down
5 changes: 2 additions & 3 deletions src/librustc_codegen_ssa/traits/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,12 @@ pub trait BuilderMethods<'a, 'tcx>:
rhs: Self::Value,
) -> (Self::Value, Self::Value);

fn alloca(&mut self, ty: Self::Type, name: &str, align: Align) -> Self::Value;
fn dynamic_alloca(&mut self, ty: Self::Type, name: &str, align: Align) -> Self::Value;
fn alloca(&mut self, ty: Self::Type, align: Align) -> Self::Value;
fn dynamic_alloca(&mut self, ty: Self::Type, align: Align) -> Self::Value;
fn array_alloca(
&mut self,
ty: Self::Type,
len: Self::Value,
name: &str,
align: Align,
) -> Self::Value;

Expand Down
2 changes: 1 addition & 1 deletion src/test/codegen/adjustments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#![crate_type = "lib"]

// Hack to get the correct size for the length part in slices
// CHECK: @helper([[USIZE:i[0-9]+]] %arg0)
// CHECK: @helper([[USIZE:i[0-9]+]] %_1)
#[no_mangle]
pub fn helper(_: usize) {
}
Expand Down
12 changes: 6 additions & 6 deletions src/test/codegen/fastcall-inreg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,27 +49,27 @@
#![crate_type = "lib"]

pub mod tests {
// CHECK: @f1(i32 inreg %arg0, i32 inreg %arg1, i32 %arg2)
// CHECK: @f1(i32 inreg %_1, i32 inreg %_2, i32 %_3)
#[no_mangle]
pub extern "fastcall" fn f1(_: i32, _: i32, _: i32) {}

// CHECK: @f2(i32* inreg %arg0, i32* inreg %arg1, i32* %arg2)
// CHECK: @f2(i32* inreg %_1, i32* inreg %_2, i32* %_3)
#[no_mangle]
pub extern "fastcall" fn f2(_: *const i32, _: *const i32, _: *const i32) {}

// CHECK: @f3(float %arg0, i32 inreg %arg1, i32 inreg %arg2, i32 %arg3)
// CHECK: @f3(float %_1, i32 inreg %_2, i32 inreg %_3, i32 %_4)
#[no_mangle]
pub extern "fastcall" fn f3(_: f32, _: i32, _: i32, _: i32) {}

// CHECK: @f4(i32 inreg %arg0, float %arg1, i32 inreg %arg2, i32 %arg3)
// CHECK: @f4(i32 inreg %_1, float %_2, i32 inreg %_3, i32 %_4)
#[no_mangle]
pub extern "fastcall" fn f4(_: i32, _: f32, _: i32, _: i32) {}

// CHECK: @f5(i64 %arg0, i32 %arg1)
// CHECK: @f5(i64 %_1, i32 %_2)
#[no_mangle]
pub extern "fastcall" fn f5(_: i64, _: i32) {}

// CHECK: @f6(i1 inreg zeroext %arg0, i32 inreg %arg1, i32 %arg2)
// CHECK: @f6(i1 inreg zeroext %_1, i32 inreg %_2, i32 %_3)
#[no_mangle]
pub extern "fastcall" fn f6(_: bool, _: i32, _: i32) {}
}
Loading

0 comments on commit 589e238

Please sign in to comment.