From 6427eaf68b5cb9f20b981c393dfd779dd6372cd5 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Fri, 30 Aug 2019 11:51:33 +0200 Subject: [PATCH 1/7] Move abi.rs to abi/mod.rs --- src/{abi.rs => abi/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/{abi.rs => abi/mod.rs} (100%) diff --git a/src/abi.rs b/src/abi/mod.rs similarity index 100% rename from src/abi.rs rename to src/abi/mod.rs From 68dcfc1c789f8d10f6d0b3981c64bca309a2481e Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Fri, 30 Aug 2019 11:58:52 +0200 Subject: [PATCH 2/7] Move pass mode handling to abi/pass_mode.rs --- src/abi/mod.rs | 188 +------------------------------------------ src/abi/pass_mode.rs | 168 ++++++++++++++++++++++++++++++++++++++ src/common.rs | 18 +++++ 3 files changed, 189 insertions(+), 185 deletions(-) create mode 100644 src/abi/pass_mode.rs diff --git a/src/abi/mod.rs b/src/abi/mod.rs index 75b35b6fac5..f359145f563 100644 --- a/src/abi/mod.rs +++ b/src/abi/mod.rs @@ -1,152 +1,11 @@ +mod pass_mode; + use std::borrow::Cow; -use rustc::ty::layout::{FloatTy, Integer, Primitive, Scalar}; use rustc_target::spec::abi::Abi; use crate::prelude::*; - -#[derive(Copy, Clone, Debug)] -enum PassMode { - NoPass, - ByVal(Type), - ByValPair(Type, Type), - ByRef, -} - -#[derive(Copy, Clone, Debug)] -enum EmptySinglePair { - Empty, - Single(T), - Pair(T, T), -} - -impl EmptySinglePair { - fn into_iter(self) -> EmptySinglePairIter { - EmptySinglePairIter(self) - } - - fn map(self, mut f: impl FnMut(T) -> U) -> EmptySinglePair { - match self { - Empty => Empty, - Single(v) => Single(f(v)), - Pair(a, b) => Pair(f(a), f(b)), - } - } -} - -struct EmptySinglePairIter(EmptySinglePair); - -impl Iterator for EmptySinglePairIter { - type Item = T; - - fn next(&mut self) -> Option { - match std::mem::replace(&mut self.0, Empty) { - Empty => None, - Single(v) => Some(v), - Pair(a, b) => { - self.0 = Single(b); - Some(a) - } - } - } -} - -impl EmptySinglePair { - fn assert_single(self) -> T { - match self { - Single(v) => v, - _ => panic!("Called assert_single on {:?}", self) - } - } - - fn assert_pair(self) -> (T, T) { - match self { - Pair(a, b) => (a, b), - _ => panic!("Called assert_pair on {:?}", self) - } - } -} - -use EmptySinglePair::*; - -impl PassMode { - fn get_param_ty(self, fx: &FunctionCx) -> EmptySinglePair { - match self { - PassMode::NoPass => Empty, - PassMode::ByVal(clif_type) => Single(clif_type), - PassMode::ByValPair(a, b) => Pair(a, b), - PassMode::ByRef => Single(fx.pointer_type), - } - } -} - -pub fn scalar_to_clif_type(tcx: TyCtxt, scalar: Scalar) -> Type { - match scalar.value { - Primitive::Int(int, _sign) => match int { - Integer::I8 => types::I8, - Integer::I16 => types::I16, - Integer::I32 => types::I32, - Integer::I64 => types::I64, - Integer::I128 => types::I128, - }, - Primitive::Float(flt) => match flt { - FloatTy::F32 => types::F32, - FloatTy::F64 => types::F64, - }, - Primitive::Pointer => pointer_ty(tcx), - } -} - -fn get_pass_mode<'tcx>( - tcx: TyCtxt<'tcx>, - layout: TyLayout<'tcx>, -) -> PassMode { - assert!(!layout.is_unsized()); - - if layout.is_zst() { - // WARNING zst arguments must never be passed, as that will break CastKind::ClosureFnPointer - PassMode::NoPass - } else { - match &layout.abi { - layout::Abi::Uninhabited => PassMode::NoPass, - layout::Abi::Scalar(scalar) => { - PassMode::ByVal(scalar_to_clif_type(tcx, scalar.clone())) - } - layout::Abi::ScalarPair(a, b) => { - let a = scalar_to_clif_type(tcx, a.clone()); - let b = scalar_to_clif_type(tcx, b.clone()); - if a == types::I128 && b == types::I128 { - // Returning (i128, i128) by-val-pair would take 4 regs, while only 3 are - // available on x86_64. Cranelift gets confused when too many return params - // are used. - PassMode::ByRef - } else { - PassMode::ByValPair(a, b) - } - } - - // FIXME implement Vector Abi in a cg_llvm compatible way - layout::Abi::Vector { .. } => PassMode::ByRef, - - layout::Abi::Aggregate { .. } => PassMode::ByRef, - } - } -} - -fn adjust_arg_for_abi<'tcx>( - fx: &mut FunctionCx<'_, 'tcx, impl Backend>, - arg: CValue<'tcx>, -) -> EmptySinglePair { - match get_pass_mode(fx.tcx, arg.layout()) { - PassMode::NoPass => Empty, - PassMode::ByVal(_) => Single(arg.load_scalar(fx)), - PassMode::ByValPair(_, _) => { - let (a, b) = arg.load_scalar_pair(fx); - Pair(a, b) - } - PassMode::ByRef => Single(arg.force_stack(fx)), - } -} +use self::pass_mode::*; fn clif_sig_from_fn_sig<'tcx>(tcx: TyCtxt<'tcx>, sig: FnSig<'tcx>, is_vtable_fn: bool) -> Signature { let abi = match sig.abi { @@ -431,47 +290,6 @@ fn local_place<'tcx>( fx.local_map[&local] } -fn cvalue_for_param<'tcx>( - fx: &mut FunctionCx<'_, 'tcx, impl Backend>, - start_ebb: Ebb, - local: mir::Local, - local_field: Option, - arg_ty: Ty<'tcx>, - ssa_flags: crate::analyze::Flags, -) -> Option> { - let layout = fx.layout_of(arg_ty); - let pass_mode = get_pass_mode(fx.tcx, fx.layout_of(arg_ty)); - - if let PassMode::NoPass = pass_mode { - return None; - } - - let clif_types = pass_mode.get_param_ty(fx); - let ebb_params = clif_types.map(|t| fx.bcx.append_ebb_param(start_ebb, t)); - - #[cfg(debug_assertions)] - add_arg_comment( - fx, - "arg", - local, - local_field, - ebb_params, - pass_mode, - ssa_flags, - arg_ty, - ); - - match pass_mode { - PassMode::NoPass => unreachable!(), - PassMode::ByVal(_) => Some(CValue::by_val(ebb_params.assert_single(), layout)), - PassMode::ByValPair(_, _) => { - let (a, b) = ebb_params.assert_pair(); - Some(CValue::by_val_pair(a, b, layout)) - } - PassMode::ByRef => Some(CValue::by_ref(ebb_params.assert_single(), layout)), - } -} - pub fn codegen_fn_prelude( fx: &mut FunctionCx<'_, '_, impl Backend>, start_ebb: Ebb, diff --git a/src/abi/pass_mode.rs b/src/abi/pass_mode.rs new file mode 100644 index 00000000000..c3a479578bc --- /dev/null +++ b/src/abi/pass_mode.rs @@ -0,0 +1,168 @@ +use crate::prelude::*; + +#[derive(Copy, Clone, Debug)] +pub enum PassMode { + NoPass, + ByVal(Type), + ByValPair(Type, Type), + ByRef, +} + +#[derive(Copy, Clone, Debug)] +pub enum EmptySinglePair { + Empty, + Single(T), + Pair(T, T), +} + +impl EmptySinglePair { + pub fn into_iter(self) -> EmptySinglePairIter { + EmptySinglePairIter(self) + } + + pub fn map(self, mut f: impl FnMut(T) -> U) -> EmptySinglePair { + match self { + Empty => Empty, + Single(v) => Single(f(v)), + Pair(a, b) => Pair(f(a), f(b)), + } + } +} + +pub struct EmptySinglePairIter(EmptySinglePair); + +impl Iterator for EmptySinglePairIter { + type Item = T; + + fn next(&mut self) -> Option { + match std::mem::replace(&mut self.0, Empty) { + Empty => None, + Single(v) => Some(v), + Pair(a, b) => { + self.0 = Single(b); + Some(a) + } + } + } +} + +impl EmptySinglePair { + pub fn assert_single(self) -> T { + match self { + Single(v) => v, + _ => panic!("Called assert_single on {:?}", self) + } + } + + pub fn assert_pair(self) -> (T, T) { + match self { + Pair(a, b) => (a, b), + _ => panic!("Called assert_pair on {:?}", self) + } + } +} + +pub use EmptySinglePair::*; + +impl PassMode { + pub fn get_param_ty(self, fx: &FunctionCx) -> EmptySinglePair { + match self { + PassMode::NoPass => Empty, + PassMode::ByVal(clif_type) => Single(clif_type), + PassMode::ByValPair(a, b) => Pair(a, b), + PassMode::ByRef => Single(fx.pointer_type), + } + } +} + +pub fn get_pass_mode<'tcx>( + tcx: TyCtxt<'tcx>, + layout: TyLayout<'tcx>, +) -> PassMode { + assert!(!layout.is_unsized()); + + if layout.is_zst() { + // WARNING zst arguments must never be passed, as that will break CastKind::ClosureFnPointer + PassMode::NoPass + } else { + match &layout.abi { + layout::Abi::Uninhabited => PassMode::NoPass, + layout::Abi::Scalar(scalar) => { + PassMode::ByVal(scalar_to_clif_type(tcx, scalar.clone())) + } + layout::Abi::ScalarPair(a, b) => { + let a = scalar_to_clif_type(tcx, a.clone()); + let b = scalar_to_clif_type(tcx, b.clone()); + if a == types::I128 && b == types::I128 { + // Returning (i128, i128) by-val-pair would take 4 regs, while only 3 are + // available on x86_64. Cranelift gets confused when too many return params + // are used. + PassMode::ByRef + } else { + PassMode::ByValPair(a, b) + } + } + + // FIXME implement Vector Abi in a cg_llvm compatible way + layout::Abi::Vector { .. } => PassMode::ByRef, + + layout::Abi::Aggregate { .. } => PassMode::ByRef, + } + } +} + +pub fn adjust_arg_for_abi<'tcx>( + fx: &mut FunctionCx<'_, 'tcx, impl Backend>, + arg: CValue<'tcx>, +) -> EmptySinglePair { + match get_pass_mode(fx.tcx, arg.layout()) { + PassMode::NoPass => Empty, + PassMode::ByVal(_) => Single(arg.load_scalar(fx)), + PassMode::ByValPair(_, _) => { + let (a, b) = arg.load_scalar_pair(fx); + Pair(a, b) + } + PassMode::ByRef => Single(arg.force_stack(fx)), + } +} + +pub fn cvalue_for_param<'tcx>( + fx: &mut FunctionCx<'_, 'tcx, impl Backend>, + start_ebb: Ebb, + local: mir::Local, + local_field: Option, + arg_ty: Ty<'tcx>, + ssa_flags: crate::analyze::Flags, +) -> Option> { + let layout = fx.layout_of(arg_ty); + let pass_mode = get_pass_mode(fx.tcx, fx.layout_of(arg_ty)); + + if let PassMode::NoPass = pass_mode { + return None; + } + + let clif_types = pass_mode.get_param_ty(fx); + let ebb_params = clif_types.map(|t| fx.bcx.append_ebb_param(start_ebb, t)); + + #[cfg(debug_assertions)] + super::add_arg_comment( + fx, + "arg", + local, + local_field, + ebb_params, + pass_mode, + ssa_flags, + arg_ty, + ); + + match pass_mode { + PassMode::NoPass => unreachable!(), + PassMode::ByVal(_) => Some(CValue::by_val(ebb_params.assert_single(), layout)), + PassMode::ByValPair(_, _) => { + let (a, b) = ebb_params.assert_pair(); + Some(CValue::by_val_pair(a, b, layout)) + } + PassMode::ByRef => Some(CValue::by_ref(ebb_params.assert_single(), layout)), + } +} diff --git a/src/common.rs b/src/common.rs index eb6f204a8d2..6ed979449fe 100644 --- a/src/common.rs +++ b/src/common.rs @@ -1,3 +1,4 @@ +use rustc::ty::layout::{FloatTy, Integer, Primitive}; use rustc_target::spec::{HasTargetSpec, Target}; use cranelift::codegen::ir::{Opcode, InstructionData, ValueDef}; @@ -17,6 +18,23 @@ pub fn pointer_ty(tcx: TyCtxt) -> types::Type { } } +pub fn scalar_to_clif_type(tcx: TyCtxt, scalar: Scalar) -> Type { + match scalar.value { + Primitive::Int(int, _sign) => match int { + Integer::I8 => types::I8, + Integer::I16 => types::I16, + Integer::I32 => types::I32, + Integer::I64 => types::I64, + Integer::I128 => types::I128, + }, + Primitive::Float(flt) => match flt { + FloatTy::F32 => types::F32, + FloatTy::F64 => types::F64, + }, + Primitive::Pointer => pointer_ty(tcx), + } +} + pub fn clif_type_from_ty<'tcx>( tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, From 16593d264c7803b78156c02b2ac596f6d98333c7 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Fri, 30 Aug 2019 12:30:57 +0200 Subject: [PATCH 3/7] Move return handling to abi/returning.rs --- src/abi/mod.rs | 142 +++++++++---------------------------------- src/abi/returning.rs | 111 +++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 112 deletions(-) create mode 100644 src/abi/returning.rs diff --git a/src/abi/mod.rs b/src/abi/mod.rs index f359145f563..a724ea2d8ec 100644 --- a/src/abi/mod.rs +++ b/src/abi/mod.rs @@ -1,3 +1,4 @@ +mod returning; mod pass_mode; use std::borrow::Cow; @@ -7,6 +8,8 @@ use rustc_target::spec::abi::Abi; use crate::prelude::*; use self::pass_mode::*; +pub use self::returning::codegen_return; + fn clif_sig_from_fn_sig<'tcx>(tcx: TyCtxt<'tcx>, sig: FnSig<'tcx>, is_vtable_fn: bool) -> Signature { let abi = match sig.abi { Abi::System => { @@ -296,35 +299,15 @@ pub fn codegen_fn_prelude( ) { let ssa_analyzed = crate::analyze::analyze(fx); - #[cfg(debug_assertions)] - fx.add_global_comment(format!("ssa {:?}", ssa_analyzed)); - - let ret_layout = fx.return_layout(); - let output_pass_mode = get_pass_mode(fx.tcx, fx.return_layout()); - let ret_param = match output_pass_mode { - PassMode::NoPass | PassMode::ByVal(_) | PassMode::ByValPair(_, _) => None, - PassMode::ByRef => Some(fx.bcx.append_ebb_param(start_ebb, fx.pointer_type)), - }; - #[cfg(debug_assertions)] { + fx.add_global_comment(format!("ssa {:?}", ssa_analyzed)); add_local_header_comment(fx); - let ret_param = match ret_param { - Some(param) => Single(param), - None => Empty, - }; - add_arg_comment( - fx, - "ret", - RETURN_PLACE, - None, - ret_param, - output_pass_mode, - ssa_analyzed[&RETURN_PLACE], - ret_layout.ty, - ); } + self::returning::codegen_return_param(fx, &ssa_analyzed, start_ebb); + + // None means pass_mode == NoPass enum ArgKind<'tcx> { Normal(Option>), @@ -373,27 +356,6 @@ pub fn codegen_fn_prelude( fx.bcx.switch_to_block(start_ebb); - match output_pass_mode { - PassMode::NoPass => { - fx.local_map - .insert(RETURN_PLACE, CPlace::no_place(ret_layout)); - } - PassMode::ByVal(_) | PassMode::ByValPair(_, _) => { - let is_ssa = !ssa_analyzed - .get(&RETURN_PLACE) - .unwrap() - .contains(crate::analyze::Flags::NOT_SSA); - - local_place(fx, RETURN_PLACE, ret_layout, is_ssa); - } - PassMode::ByRef => { - fx.local_map.insert( - RETURN_PLACE, - CPlace::for_addr(ret_param.unwrap(), ret_layout), - ); - } - } - for (local, arg_kind, ty) in func_params { let layout = fx.layout_of(ty); @@ -524,18 +486,6 @@ fn codegen_call_inner<'tcx>( ) { let fn_sig = fx.tcx.normalize_erasing_late_bound_regions(ParamEnv::reveal_all(), &fn_ty.fn_sig(fx.tcx)); - let ret_layout = fx.layout_of(fn_sig.output()); - - let output_pass_mode = get_pass_mode(fx.tcx, fx.layout_of(fn_sig.output())); - let return_ptr = match output_pass_mode { - PassMode::NoPass => None, - PassMode::ByRef => match ret_place { - Some(ret_place) => Some(ret_place.to_addr(fx)), - None => Some(fx.bcx.ins().iconst(fx.pointer_type, 43)), - }, - PassMode::ByVal(_) | PassMode::ByValPair(_, _) => None, - }; - let instance = match fn_ty.sty { ty::FnDef(def_id, substs) => { Some(Instance::resolve(fx.tcx, ParamEnv::reveal_all(), def_id, substs).unwrap()) @@ -584,26 +534,30 @@ fn codegen_call_inner<'tcx>( } }; - let call_args: Vec = return_ptr - .into_iter() - .chain(first_arg.into_iter()) - .chain( - args.into_iter() - .skip(1) - .map(|arg| adjust_arg_for_abi(fx, arg).into_iter()) - .flatten(), - ) - .collect::>(); + let (call_inst, call_args) = self::returning::codegen_with_call_return_arg(fx, fn_sig, ret_place, |fx, return_ptr| { + let call_args: Vec = return_ptr + .into_iter() + .chain(first_arg.into_iter()) + .chain( + args.into_iter() + .skip(1) + .map(|arg| adjust_arg_for_abi(fx, arg).into_iter()) + .flatten(), + ) + .collect::>(); - let call_inst = if let Some(func_ref) = func_ref { - let sig = fx - .bcx - .import_signature(clif_sig_from_fn_sig(fx.tcx, fn_sig, is_virtual_call)); - fx.bcx.ins().call_indirect(sig, func_ref, &call_args) - } else { - let func_ref = fx.get_function_ref(instance.expect("non-indirect call on non-FnDef type")); - fx.bcx.ins().call(func_ref, &call_args) - }; + let call_inst = if let Some(func_ref) = func_ref { + let sig = fx + .bcx + .import_signature(clif_sig_from_fn_sig(fx.tcx, fn_sig, is_virtual_call)); + fx.bcx.ins().call_indirect(sig, func_ref, &call_args) + } else { + let func_ref = fx.get_function_ref(instance.expect("non-indirect call on non-FnDef type")); + fx.bcx.ins().call(func_ref, &call_args) + }; + + (call_inst, call_args) + }); // FIXME find a cleaner way to support varargs if fn_sig.c_variadic { @@ -624,24 +578,6 @@ fn codegen_call_inner<'tcx>( .collect::>(); fx.bcx.func.dfg.signatures[sig_ref].params = abi_params; } - - match output_pass_mode { - PassMode::NoPass => {} - PassMode::ByVal(_) => { - if let Some(ret_place) = ret_place { - let ret_val = fx.bcx.inst_results(call_inst)[0]; - ret_place.write_cvalue(fx, CValue::by_val(ret_val, ret_layout)); - } - } - PassMode::ByValPair(_, _) => { - if let Some(ret_place) = ret_place { - let ret_val_a = fx.bcx.inst_results(call_inst)[0]; - let ret_val_b = fx.bcx.inst_results(call_inst)[1]; - ret_place.write_cvalue(fx, CValue::by_val_pair(ret_val_a, ret_val_b, ret_layout)); - } - } - PassMode::ByRef => {} - } } pub fn codegen_drop<'tcx>( @@ -693,21 +629,3 @@ pub fn codegen_drop<'tcx>( } } } - -pub fn codegen_return(fx: &mut FunctionCx) { - match get_pass_mode(fx.tcx, fx.return_layout()) { - PassMode::NoPass | PassMode::ByRef => { - fx.bcx.ins().return_(&[]); - } - PassMode::ByVal(_) => { - let place = fx.get_local_place(RETURN_PLACE); - let ret_val = place.to_cvalue(fx).load_scalar(fx); - fx.bcx.ins().return_(&[ret_val]); - } - PassMode::ByValPair(_, _) => { - let place = fx.get_local_place(RETURN_PLACE); - let (ret_val_a, ret_val_b) = place.to_cvalue(fx).load_scalar_pair(fx); - fx.bcx.ins().return_(&[ret_val_a, ret_val_b]); - } - } -} diff --git a/src/abi/returning.rs b/src/abi/returning.rs new file mode 100644 index 00000000000..45ba48d7c4f --- /dev/null +++ b/src/abi/returning.rs @@ -0,0 +1,111 @@ +use crate::prelude::*; +use crate::abi::pass_mode::*; + +pub fn codegen_return_param( + fx: &mut FunctionCx, + ssa_analyzed: &HashMap, + start_ebb: Ebb, +) { + let ret_layout = fx.return_layout(); + let output_pass_mode = get_pass_mode(fx.tcx, fx.return_layout()); + + let ret_param = match output_pass_mode { + PassMode::NoPass => { + fx.local_map + .insert(RETURN_PLACE, CPlace::no_place(ret_layout)); + Empty + } + PassMode::ByVal(_) | PassMode::ByValPair(_, _) => { + let is_ssa = !ssa_analyzed + .get(&RETURN_PLACE) + .unwrap() + .contains(crate::analyze::Flags::NOT_SSA); + + super::local_place(fx, RETURN_PLACE, ret_layout, is_ssa); + + Empty + } + PassMode::ByRef => { + let ret_param = fx.bcx.append_ebb_param(start_ebb, fx.pointer_type); + fx.local_map.insert( + RETURN_PLACE, + CPlace::for_addr(ret_param, ret_layout), + ); + + Single(ret_param) + } + }; + + #[cfg(debug_assertions)] + { + super::add_arg_comment( + fx, + "ret", + RETURN_PLACE, + None, + ret_param, + output_pass_mode, + ssa_analyzed[&RETURN_PLACE], + ret_layout.ty, + ); + } +} + +pub fn codegen_with_call_return_arg<'tcx, B: Backend, T>( + fx: &mut FunctionCx<'_, 'tcx, B>, + fn_sig: FnSig<'tcx>, + ret_place: Option>, + f: impl FnOnce(&mut FunctionCx<'_, 'tcx, B>, Option) -> (Inst, T), +) -> (Inst, T) { + let ret_layout = fx.layout_of(fn_sig.output()); + + let output_pass_mode = get_pass_mode(fx.tcx, ret_layout); + let return_ptr = match output_pass_mode { + PassMode::NoPass => None, + PassMode::ByRef => match ret_place { + Some(ret_place) => Some(ret_place.to_addr(fx)), + None => Some(fx.bcx.ins().iconst(fx.pointer_type, 43)), + }, + PassMode::ByVal(_) | PassMode::ByValPair(_, _) => None, + }; + + let (call_inst, meta) = f(fx, return_ptr); + + match output_pass_mode { + PassMode::NoPass => {} + PassMode::ByVal(_) => { + if let Some(ret_place) = ret_place { + let ret_val = fx.bcx.inst_results(call_inst)[0]; + ret_place.write_cvalue(fx, CValue::by_val(ret_val, ret_layout)); + } + } + PassMode::ByValPair(_, _) => { + if let Some(ret_place) = ret_place { + let ret_val_a = fx.bcx.inst_results(call_inst)[0]; + let ret_val_b = fx.bcx.inst_results(call_inst)[1]; + ret_place.write_cvalue(fx, CValue::by_val_pair(ret_val_a, ret_val_b, ret_layout)); + } + } + PassMode::ByRef => {} + } + + (call_inst, meta) +} + +pub fn codegen_return(fx: &mut FunctionCx) { + match get_pass_mode(fx.tcx, fx.return_layout()) { + PassMode::NoPass | PassMode::ByRef => { + fx.bcx.ins().return_(&[]); + } + PassMode::ByVal(_) => { + let place = fx.get_local_place(RETURN_PLACE); + let ret_val = place.to_cvalue(fx).load_scalar(fx); + fx.bcx.ins().return_(&[ret_val]); + } + PassMode::ByValPair(_, _) => { + let place = fx.get_local_place(RETURN_PLACE); + let (ret_val_a, ret_val_b) = place.to_cvalue(fx).load_scalar_pair(fx); + fx.bcx.ins().return_(&[ret_val_a, ret_val_b]); + } + } +} From deeae2fce46cba99a2dcce835b85f7d42238c3c8 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Fri, 30 Aug 2019 12:42:18 +0200 Subject: [PATCH 4/7] Small change --- src/abi/mod.rs | 7 +------ src/abi/pass_mode.rs | 6 +++--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/abi/mod.rs b/src/abi/mod.rs index a724ea2d8ec..dc6bac0e426 100644 --- a/src/abi/mod.rs +++ b/src/abi/mod.rs @@ -49,12 +49,7 @@ fn clif_sig_from_fn_sig<'tcx>(tcx: TyCtxt<'tcx>, sig: FnSig<'tcx>, is_vtable_fn: // See https://github.com/rust-lang/rust/blob/37b6a5e5e82497caf5353d9d856e4eb5d14cbe06/src/librustc/ty/layout.rs#L2519-L2572 for more info layout = tcx.layout_of(ParamEnv::reveal_all().and(tcx.mk_mut_ptr(tcx.mk_unit()))).unwrap(); } - match get_pass_mode(tcx, layout) { - PassMode::NoPass => Empty, - PassMode::ByVal(clif_ty) => Single(clif_ty), - PassMode::ByValPair(clif_ty_a, clif_ty_b) => Pair(clif_ty_a, clif_ty_b), - PassMode::ByRef => Single(pointer_ty(tcx)), - }.into_iter() + get_pass_mode(tcx, layout).get_param_ty(tcx).into_iter() }).flatten(); let (params, returns) = match get_pass_mode(tcx, tcx.layout_of(ParamEnv::reveal_all().and(output)).unwrap()) { diff --git a/src/abi/pass_mode.rs b/src/abi/pass_mode.rs index c3a479578bc..4608562e993 100644 --- a/src/abi/pass_mode.rs +++ b/src/abi/pass_mode.rs @@ -65,12 +65,12 @@ impl EmptySinglePair { pub use EmptySinglePair::*; impl PassMode { - pub fn get_param_ty(self, fx: &FunctionCx) -> EmptySinglePair { + pub fn get_param_ty(self, tcx: TyCtxt<'_>) -> EmptySinglePair { match self { PassMode::NoPass => Empty, PassMode::ByVal(clif_type) => Single(clif_type), PassMode::ByValPair(a, b) => Pair(a, b), - PassMode::ByRef => Single(fx.pointer_type), + PassMode::ByRef => Single(pointer_ty(tcx)), } } } @@ -141,7 +141,7 @@ pub fn cvalue_for_param<'tcx>( return None; } - let clif_types = pass_mode.get_param_ty(fx); + let clif_types = pass_mode.get_param_ty(fx.tcx); let ebb_params = clif_types.map(|t| fx.bcx.append_ebb_param(start_ebb, t)); #[cfg(debug_assertions)] From d731c4a6a7594cdcbe665b40b4690e109c60bdbf Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Fri, 30 Aug 2019 14:21:24 +0200 Subject: [PATCH 5/7] Move ir comments generation to abi/comments.rs Also list locals stored in ssa vars in the comments --- src/abi/comments.rs | 94 ++++++++++++++++++++++++++++++++++++++++++++ src/abi/mod.rs | 92 ++++--------------------------------------- src/abi/pass_mode.rs | 2 +- src/abi/returning.rs | 22 +++++------ 4 files changed, 112 insertions(+), 98 deletions(-) create mode 100644 src/abi/comments.rs diff --git a/src/abi/comments.rs b/src/abi/comments.rs new file mode 100644 index 00000000000..674449cf06d --- /dev/null +++ b/src/abi/comments.rs @@ -0,0 +1,94 @@ +use std::borrow::Cow; + +use rustc::mir; + +use crate::prelude::*; +use crate::abi::pass_mode::*; + +pub fn add_local_header_comment(fx: &mut FunctionCx) { + fx.add_global_comment(format!( + "msg loc.idx param pass mode ssa flags ty" + )); +} + +pub fn add_arg_comment<'tcx>( + fx: &mut FunctionCx<'_, 'tcx, impl Backend>, + msg: &str, + local: mir::Local, + local_field: Option, + params: EmptySinglePair, + pass_mode: PassMode, + ssa: crate::analyze::Flags, + ty: Ty<'tcx>, +) { + let local_field = if let Some(local_field) = local_field { + Cow::Owned(format!(".{}", local_field)) + } else { + Cow::Borrowed("") + }; + let params = match params { + Empty => Cow::Borrowed("-"), + Single(param) => Cow::Owned(format!("= {:?}", param)), + Pair(param_a, param_b) => Cow::Owned(format!("= {:?}, {:?}", param_a, param_b)), + }; + let pass_mode = format!("{:?}", pass_mode); + fx.add_global_comment(format!( + "{msg:5}{local:>3}{local_field:<5} {params:10} {pass_mode:36} {ssa:10} {ty:?}", + msg = msg, + local = format!("{:?}", local), + local_field = local_field, + params = params, + pass_mode = pass_mode, + ssa = format!("{:?}", ssa), + ty = ty, + )); +} + +pub fn add_local_place_comments<'tcx>( + fx: &mut FunctionCx<'_, 'tcx, impl Backend>, + place: CPlace<'tcx>, + local: Local, +) { + let TyLayout { ty, details } = place.layout(); + let ty::layout::LayoutDetails { + size, + align, + abi: _, + variants: _, + fields: _, + largest_niche: _, + } = details; + match *place.inner() { + CPlaceInner::Var(var) => { + assert_eq!(local, var); + fx.add_global_comment(format!( + "ssa {:?}: {:?} size={} align={}, {}", + local, + ty, + size.bytes(), + align.abi.bytes(), + align.pref.bytes(), + )); + } + CPlaceInner::Stack(stack_slot) => fx.add_entity_comment( + stack_slot, + format!( + "{:?}: {:?} size={} align={},{}", + local, + ty, + size.bytes(), + align.abi.bytes(), + align.pref.bytes(), + ), + ), + CPlaceInner::NoPlace => fx.add_global_comment(format!( + "zst {:?}: {:?} size={} align={}, {}", + local, + ty, + size.bytes(), + align.abi.bytes(), + align.pref.bytes(), + )), + CPlaceInner::Addr(_, _) => unreachable!(), + } +} diff --git a/src/abi/mod.rs b/src/abi/mod.rs index dc6bac0e426..512b1b68ad9 100644 --- a/src/abi/mod.rs +++ b/src/abi/mod.rs @@ -1,8 +1,8 @@ +#[cfg(debug_assertions)] +mod comments; mod returning; mod pass_mode; -use std::borrow::Cow; - use rustc_target::spec::abi::Abi; use crate::prelude::*; @@ -193,47 +193,6 @@ impl<'tcx, B: Backend + 'static> FunctionCx<'_, 'tcx, B> { } } -#[cfg(debug_assertions)] -fn add_arg_comment<'tcx>( - fx: &mut FunctionCx<'_, 'tcx, impl Backend>, - msg: &str, - local: mir::Local, - local_field: Option, - params: EmptySinglePair, - pass_mode: PassMode, - ssa: crate::analyze::Flags, - ty: Ty<'tcx>, -) { - let local_field = if let Some(local_field) = local_field { - Cow::Owned(format!(".{}", local_field)) - } else { - Cow::Borrowed("") - }; - let params = match params { - Empty => Cow::Borrowed("-"), - Single(param) => Cow::Owned(format!("= {:?}", param)), - Pair(param_a, param_b) => Cow::Owned(format!("= {:?}, {:?}", param_a, param_b)), - }; - let pass_mode = format!("{:?}", pass_mode); - fx.add_global_comment(format!( - "{msg:5} {local:>3}{local_field:<5} {params:10} {pass_mode:36} {ssa:10} {ty:?}", - msg = msg, - local = format!("{:?}", local), - local_field = local_field, - params = params, - pass_mode = pass_mode, - ssa = format!("{:?}", ssa), - ty = ty, - )); -} - -#[cfg(debug_assertions)] -fn add_local_header_comment(fx: &mut FunctionCx) { - fx.add_global_comment(format!( - "msg loc.idx param pass mode ssa flags ty" - )); -} - fn local_place<'tcx>( fx: &mut FunctionCx<'_, 'tcx, impl Backend>, local: Local, @@ -243,46 +202,12 @@ fn local_place<'tcx>( let place = if is_ssa { CPlace::new_var(fx, local, layout) } else { - let place = CPlace::new_stack_slot(fx, layout.ty); - - #[cfg(debug_assertions)] - { - let TyLayout { ty, details } = layout; - let ty::layout::LayoutDetails { - size, - align, - abi: _, - variants: _, - fields: _, - largest_niche: _, - } = details; - match *place.inner() { - CPlaceInner::Stack(stack_slot) => fx.add_entity_comment( - stack_slot, - format!( - "{:?}: {:?} size={} align={},{}", - local, - ty, - size.bytes(), - align.abi.bytes(), - align.pref.bytes(), - ), - ), - CPlaceInner::NoPlace => fx.add_global_comment(format!( - "zst {:?}: {:?} size={} align={}, {}", - local, - ty, - size.bytes(), - align.abi.bytes(), - align.pref.bytes(), - )), - _ => unreachable!(), - } - } - - place + CPlace::new_stack_slot(fx, layout.ty) }; + #[cfg(debug_assertions)] + self::comments::add_local_place_comments(fx, place, local); + let prev_place = fx.local_map.insert(local, place); debug_assert!(prev_place.is_none()); fx.local_map[&local] @@ -295,10 +220,7 @@ pub fn codegen_fn_prelude( let ssa_analyzed = crate::analyze::analyze(fx); #[cfg(debug_assertions)] - { - fx.add_global_comment(format!("ssa {:?}", ssa_analyzed)); - add_local_header_comment(fx); - } + self::comments::add_local_header_comment(fx); self::returning::codegen_return_param(fx, &ssa_analyzed, start_ebb); diff --git a/src/abi/pass_mode.rs b/src/abi/pass_mode.rs index 4608562e993..f5c91338842 100644 --- a/src/abi/pass_mode.rs +++ b/src/abi/pass_mode.rs @@ -145,7 +145,7 @@ pub fn cvalue_for_param<'tcx>( let ebb_params = clif_types.map(|t| fx.bcx.append_ebb_param(start_ebb, t)); #[cfg(debug_assertions)] - super::add_arg_comment( + crate::abi::comments::add_arg_comment( fx, "arg", local, diff --git a/src/abi/returning.rs b/src/abi/returning.rs index 45ba48d7c4f..465127e9b71 100644 --- a/src/abi/returning.rs +++ b/src/abi/returning.rs @@ -37,18 +37,16 @@ pub fn codegen_return_param( }; #[cfg(debug_assertions)] - { - super::add_arg_comment( - fx, - "ret", - RETURN_PLACE, - None, - ret_param, - output_pass_mode, - ssa_analyzed[&RETURN_PLACE], - ret_layout.ty, - ); - } + crate::abi::comments::add_arg_comment( + fx, + "ret", + RETURN_PLACE, + None, + ret_param, + output_pass_mode, + ssa_analyzed[&RETURN_PLACE], + ret_layout.ty, + ); } pub fn codegen_with_call_return_arg<'tcx, B: Backend, T>( From 76d2e085db71370cb34fc12fc544f46e557e743d Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Fri, 30 Aug 2019 15:07:15 +0200 Subject: [PATCH 6/7] Improve abi ir comments a bit --- src/abi/comments.rs | 31 ++++++++++++++++++------------- src/abi/mod.rs | 9 +++++---- src/abi/pass_mode.rs | 2 -- src/abi/returning.rs | 1 - 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/abi/comments.rs b/src/abi/comments.rs index 674449cf06d..b7932e2ba58 100644 --- a/src/abi/comments.rs +++ b/src/abi/comments.rs @@ -5,20 +5,19 @@ use rustc::mir; use crate::prelude::*; use crate::abi::pass_mode::*; -pub fn add_local_header_comment(fx: &mut FunctionCx) { +pub fn add_args_header_comment(fx: &mut FunctionCx) { fx.add_global_comment(format!( - "msg loc.idx param pass mode ssa flags ty" + "kind loc.idx param pass mode ty" )); } pub fn add_arg_comment<'tcx>( fx: &mut FunctionCx<'_, 'tcx, impl Backend>, - msg: &str, + kind: &str, local: mir::Local, local_field: Option, params: EmptySinglePair, pass_mode: PassMode, - ssa: crate::analyze::Flags, ty: Ty<'tcx>, ) { let local_field = if let Some(local_field) = local_field { @@ -33,17 +32,23 @@ pub fn add_arg_comment<'tcx>( }; let pass_mode = format!("{:?}", pass_mode); fx.add_global_comment(format!( - "{msg:5}{local:>3}{local_field:<5} {params:10} {pass_mode:36} {ssa:10} {ty:?}", - msg = msg, + "{kind:5}{local:>3}{local_field:<5} {params:10} {pass_mode:36} {ty:?}", + kind = kind, local = format!("{:?}", local), local_field = local_field, params = params, pass_mode = pass_mode, - ssa = format!("{:?}", ssa), ty = ty, )); } +pub fn add_locals_header_comment(fx: &mut FunctionCx) { + fx.add_global_comment(String::new()); + fx.add_global_comment(format!( + "kind local ty size align (abi,pref)" + )); +} + pub fn add_local_place_comments<'tcx>( fx: &mut FunctionCx<'_, 'tcx, impl Backend>, place: CPlace<'tcx>, @@ -62,9 +67,9 @@ pub fn add_local_place_comments<'tcx>( CPlaceInner::Var(var) => { assert_eq!(local, var); fx.add_global_comment(format!( - "ssa {:?}: {:?} size={} align={}, {}", - local, - ty, + "ssa {:5} {:20} {:4}b {}, {}", + format!("{:?}", local), + format!("{:?}", ty), size.bytes(), align.abi.bytes(), align.pref.bytes(), @@ -82,9 +87,9 @@ pub fn add_local_place_comments<'tcx>( ), ), CPlaceInner::NoPlace => fx.add_global_comment(format!( - "zst {:?}: {:?} size={} align={}, {}", - local, - ty, + "zst {:5} {:20} {:4}b {}, {}", + format!("{:?}", local), + format!("{:?}", ty), size.bytes(), align.abi.bytes(), align.pref.bytes(), diff --git a/src/abi/mod.rs b/src/abi/mod.rs index 512b1b68ad9..eaf25ef48ec 100644 --- a/src/abi/mod.rs +++ b/src/abi/mod.rs @@ -220,11 +220,10 @@ pub fn codegen_fn_prelude( let ssa_analyzed = crate::analyze::analyze(fx); #[cfg(debug_assertions)] - self::comments::add_local_header_comment(fx); + self::comments::add_args_header_comment(fx); self::returning::codegen_return_param(fx, &ssa_analyzed, start_ebb); - // None means pass_mode == NoPass enum ArgKind<'tcx> { Normal(Option>), @@ -257,7 +256,6 @@ pub fn codegen_fn_prelude( local, Some(i), arg_ty, - ssa_analyzed[&local], ); params.push(param); } @@ -265,7 +263,7 @@ pub fn codegen_fn_prelude( (local, ArgKind::Spread(params), arg_ty) } else { let param = - cvalue_for_param(fx, start_ebb, local, None, arg_ty, ssa_analyzed[&local]); + cvalue_for_param(fx, start_ebb, local, None, arg_ty); (local, ArgKind::Normal(param), arg_ty) } }) @@ -273,6 +271,9 @@ pub fn codegen_fn_prelude( fx.bcx.switch_to_block(start_ebb); + #[cfg(debug_assertions)] + self::comments::add_locals_header_comment(fx); + for (local, arg_kind, ty) in func_params { let layout = fx.layout_of(ty); diff --git a/src/abi/pass_mode.rs b/src/abi/pass_mode.rs index f5c91338842..411ca23a1f7 100644 --- a/src/abi/pass_mode.rs +++ b/src/abi/pass_mode.rs @@ -132,7 +132,6 @@ pub fn cvalue_for_param<'tcx>( local: mir::Local, local_field: Option, arg_ty: Ty<'tcx>, - ssa_flags: crate::analyze::Flags, ) -> Option> { let layout = fx.layout_of(arg_ty); let pass_mode = get_pass_mode(fx.tcx, fx.layout_of(arg_ty)); @@ -152,7 +151,6 @@ pub fn cvalue_for_param<'tcx>( local_field, ebb_params, pass_mode, - ssa_flags, arg_ty, ); diff --git a/src/abi/returning.rs b/src/abi/returning.rs index 465127e9b71..3d34dbf06e2 100644 --- a/src/abi/returning.rs +++ b/src/abi/returning.rs @@ -44,7 +44,6 @@ pub fn codegen_return_param( None, ret_param, output_pass_mode, - ssa_analyzed[&RETURN_PLACE], ret_layout.ty, ); } From 15b9834d7d37d601fd77db11f8852f9ceb0804d0 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Fri, 30 Aug 2019 15:41:33 +0200 Subject: [PATCH 7/7] Don't copy ByRef passed types to local stack slot when not necessary Eg when the local is immutable **and** the type is freeze. This makes the simple raytracer runtime benchmark 1% faster than cg_llvm without optimizations. Before it was 2% slower. cc #691 cc #684 --- example/example.rs | 4 ++++ src/abi/comments.rs | 11 ++++++++++- src/abi/mod.rs | 28 ++++++++++++++++++++++++++++ src/value_and_place.rs | 7 +++++++ 4 files changed, 49 insertions(+), 1 deletion(-) diff --git a/example/example.rs b/example/example.rs index e8ddc4aaea1..5878e8548d9 100644 --- a/example/example.rs +++ b/example/example.rs @@ -202,3 +202,7 @@ fn get_sized_field_ref_from_unsized_type(u: &Unsized) -> &u8 { fn get_unsized_field_ref_from_unsized_type(u: &Unsized) -> &str { &u.1 } + +pub fn reuse_byref_argument_storage(a: (u8, u16, u32)) -> u8 { + a.0 +} diff --git a/src/abi/comments.rs b/src/abi/comments.rs index b7932e2ba58..77649e40b19 100644 --- a/src/abi/comments.rs +++ b/src/abi/comments.rs @@ -94,6 +94,15 @@ pub fn add_local_place_comments<'tcx>( align.abi.bytes(), align.pref.bytes(), )), - CPlaceInner::Addr(_, _) => unreachable!(), + CPlaceInner::Addr(addr, None) => fx.add_global_comment(format!( + "reuse {:5} {:20} {:4}b {}, {} storage={}", + format!("{:?}", local), + format!("{:?}", ty), + size.bytes(), + align.abi.bytes(), + align.pref.bytes(), + addr, + )), + CPlaceInner::Addr(_, Some(_)) => unreachable!(), } } diff --git a/src/abi/mod.rs b/src/abi/mod.rs index eaf25ef48ec..45277694702 100644 --- a/src/abi/mod.rs +++ b/src/abi/mod.rs @@ -282,6 +282,34 @@ pub fn codegen_fn_prelude( .unwrap() .contains(crate::analyze::Flags::NOT_SSA); + match arg_kind { + ArgKind::Normal(Some(val)) => { + if let Some(addr) = val.try_to_addr() { + let local_decl = &fx.mir.local_decls[local]; + // v this ! is important + let internally_mutable = !val.layout().ty.is_freeze( + fx.tcx, + ParamEnv::reveal_all(), + local_decl.source_info.span, + ); + if local_decl.mutability == mir::Mutability::Not && internally_mutable { + // We wont mutate this argument, so it is fine to borrow the backing storage + // of this argument, to prevent a copy. + + let place = CPlace::for_addr(addr, val.layout()); + + #[cfg(debug_assertions)] + self::comments::add_local_place_comments(fx, place, local); + + let prev_place = fx.local_map.insert(local, place); + debug_assert!(prev_place.is_none()); + continue; + } + } + } + _ => {} + } + let place = local_place(fx, local, layout, is_ssa); match arg_kind { diff --git a/src/value_and_place.rs b/src/value_and_place.rs index b300db2ecfc..b1da6e2b4ea 100644 --- a/src/value_and_place.rs +++ b/src/value_and_place.rs @@ -63,6 +63,13 @@ impl<'tcx> CValue<'tcx> { } } + pub fn try_to_addr(self) -> Option { + match self.0 { + CValueInner::ByRef(addr) => Some(addr), + CValueInner::ByVal(_) | CValueInner::ByValPair(_, _) => None, + } + } + /// Load a value with layout.abi of scalar pub fn load_scalar<'a>(self, fx: &mut FunctionCx<'_, 'tcx, impl Backend>) -> Value { let layout = self.1;