From 967a228208a88791c66a4ee3ee72cc75fa2b7fbc Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sat, 14 Nov 2020 12:27:57 +0100 Subject: [PATCH 1/5] Replace ZExt and SExt flags with ArgExtension enum Both flags are mutually exclusive --- compiler/rustc_codegen_llvm/src/abi.rs | 20 +++++++++++- compiler/rustc_middle/src/ty/layout.rs | 2 +- compiler/rustc_target/src/abi/call/mips64.rs | 4 +-- compiler/rustc_target/src/abi/call/mod.rs | 32 ++++++++++++++++++-- compiler/rustc_target/src/abi/call/riscv.rs | 4 +-- 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index 7857ccb613bf8..3ca0a9ce8d983 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -36,7 +36,7 @@ impl ArgAttributeExt for ArgAttribute { where F: FnMut(llvm::Attribute), { - for_each_kind!(self, f, NoAlias, NoCapture, NonNull, ReadOnly, SExt, StructRet, ZExt, InReg) + for_each_kind!(self, f, NoAlias, NoCapture, NonNull, ReadOnly, StructRet, InReg) } } @@ -65,6 +65,15 @@ impl ArgAttributesExt for ArgAttributes { llvm::LLVMRustAddByValAttr(llfn, idx.as_uint(), ty.unwrap()); } regular.for_each_kind(|attr| attr.apply_llfn(idx, llfn)); + match self.arg_ext { + ArgExtension::None => {} + ArgExtension::Zext => { + llvm::Attribute::ZExt.apply_llfn(idx, llfn); + } + ArgExtension::Sext => { + llvm::Attribute::SExt.apply_llfn(idx, llfn); + } + } } } @@ -95,6 +104,15 @@ impl ArgAttributesExt for ArgAttributes { llvm::LLVMRustAddByValCallSiteAttr(callsite, idx.as_uint(), ty.unwrap()); } regular.for_each_kind(|attr| attr.apply_callsite(idx, callsite)); + match self.arg_ext { + ArgExtension::None => {} + ArgExtension::Zext => { + llvm::Attribute::ZExt.apply_callsite(idx, callsite); + } + ArgExtension::Sext => { + llvm::Attribute::SExt.apply_callsite(idx, callsite); + } + } } } } diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 5626c864fe175..32bd9651ece99 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2619,7 +2619,7 @@ where is_return: bool| { // Booleans are always an i1 that needs to be zero-extended. if scalar.is_bool() { - attrs.set(ArgAttribute::ZExt); + attrs.zext(); return; } diff --git a/compiler/rustc_target/src/abi/call/mips64.rs b/compiler/rustc_target/src/abi/call/mips64.rs index 917dd104d1496..739ec6112ad3b 100644 --- a/compiler/rustc_target/src/abi/call/mips64.rs +++ b/compiler/rustc_target/src/abi/call/mips64.rs @@ -1,4 +1,4 @@ -use crate::abi::call::{ArgAbi, ArgAttribute, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform}; +use crate::abi::call::{ArgAbi, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform}; use crate::abi::{self, HasDataLayout, LayoutOf, Size, TyAndLayout, TyAndLayoutMethods}; fn extend_integer_width_mips(arg: &mut ArgAbi<'_, Ty>, bits: u64) { @@ -7,7 +7,7 @@ fn extend_integer_width_mips(arg: &mut ArgAbi<'_, Ty>, bits: u64) { if let abi::Int(i, signed) = scalar.value { if !signed && i.size().bits() == 32 { if let PassMode::Direct(ref mut attrs) = arg.mode { - attrs.set(ArgAttribute::SExt); + attrs.sext(); return; } } diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 429a3375cd893..9d657250c7c79 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -57,19 +57,28 @@ mod attr_impl { const NoCapture = 1 << 2; const NonNull = 1 << 3; const ReadOnly = 1 << 4; - const SExt = 1 << 5; const StructRet = 1 << 6; - const ZExt = 1 << 7; const InReg = 1 << 8; } } } +/// Sometimes an ABI requires small integers to be extended to a full or partial register. This enum +/// defines if this extension should be zero-extension or sign-extension when necssary. When it is +/// not necesary to extend the argument, this enum is ignored. +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +pub enum ArgExtension { + None, + Zext, + Sext, +} + /// A compact representation of LLVM attributes (at least those relevant for this module) /// that can be manipulated without interacting with LLVM's Attribute machinery. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct ArgAttributes { pub regular: ArgAttribute, + pub arg_ext: ArgExtension, /// The minimum size of the pointee, guaranteed to be valid for the duration of the whole call /// (corresponding to LLVM's dereferenceable and dereferenceable_or_null attributes). pub pointee_size: Size, @@ -80,11 +89,24 @@ impl ArgAttributes { pub fn new() -> Self { ArgAttributes { regular: ArgAttribute::default(), + arg_ext: ArgExtension::None, pointee_size: Size::ZERO, pointee_align: None, } } + pub fn zext(&mut self) -> &mut Self { + assert_ne!(self.arg_ext, ArgExtension::Sext); + self.arg_ext = ArgExtension::Zext; + self + } + + pub fn sext(&mut self) -> &mut Self { + assert_ne!(self.arg_ext, ArgExtension::Zext); + self.arg_ext = ArgExtension::Sext; + self + } + pub fn set(&mut self, attr: ArgAttribute) -> &mut Self { self.regular |= attr; self @@ -457,7 +479,11 @@ impl<'a, Ty> ArgAbi<'a, Ty> { if let abi::Int(i, signed) = scalar.value { if i.size().bits() < bits { if let PassMode::Direct(ref mut attrs) = self.mode { - attrs.set(if signed { ArgAttribute::SExt } else { ArgAttribute::ZExt }); + if signed { + attrs.sext() + } else { + attrs.zext() + }; } } } diff --git a/compiler/rustc_target/src/abi/call/riscv.rs b/compiler/rustc_target/src/abi/call/riscv.rs index 782c661c31fdb..9c4ac85ce6135 100644 --- a/compiler/rustc_target/src/abi/call/riscv.rs +++ b/compiler/rustc_target/src/abi/call/riscv.rs @@ -4,7 +4,7 @@ // Reference: Clang RISC-V ELF psABI lowering code // https://github.com/llvm/llvm-project/blob/8e780252a7284be45cf1ba224cabd884847e8e92/clang/lib/CodeGen/TargetInfo.cpp#L9311-L9773 -use crate::abi::call::{ArgAbi, ArgAttribute, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform}; +use crate::abi::call::{ArgAbi, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform}; use crate::abi::{ self, Abi, FieldsShape, HasDataLayout, LayoutOf, Size, TyAndLayout, TyAndLayoutMethods, }; @@ -308,7 +308,7 @@ fn extend_integer_width<'a, Ty>(arg: &mut ArgAbi<'a, Ty>, xlen: u64) { // 32-bit integers are always sign-extended if i.size().bits() == 32 && xlen > 32 { if let PassMode::Direct(ref mut attrs) = arg.mode { - attrs.set(ArgAttribute::SExt); + attrs.sext(); return; } } From 42b0b8080d10b412d51d414a0de0e08959a64f5f Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sat, 14 Nov 2020 14:29:40 +0100 Subject: [PATCH 2/5] Replace ByVal attribute with on_stack field for Indirect This makes it clearer that only PassMode::Indirect allows ByVal --- compiler/rustc_codegen_llvm/src/abi.rs | 121 +++++++++++++------- compiler/rustc_codegen_ssa/src/mir/block.rs | 4 +- compiler/rustc_middle/src/ty/layout.rs | 3 +- compiler/rustc_target/src/abi/call/mod.rs | 22 ++-- compiler/rustc_target/src/abi/call/x86.rs | 9 +- 5 files changed, 102 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index 3ca0a9ce8d983..e988fdb9d5c53 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -41,12 +41,12 @@ impl ArgAttributeExt for ArgAttribute { } pub trait ArgAttributesExt { - fn apply_llfn(&self, idx: AttributePlace, llfn: &Value, ty: Option<&Type>); - fn apply_callsite(&self, idx: AttributePlace, callsite: &Value, ty: Option<&Type>); + fn apply_attrs_to_llfn(&self, idx: AttributePlace, llfn: &Value); + fn apply_attrs_to_callsite(&self, idx: AttributePlace, callsite: &Value); } impl ArgAttributesExt for ArgAttributes { - fn apply_llfn(&self, idx: AttributePlace, llfn: &Value, ty: Option<&Type>) { + fn apply_attrs_to_llfn(&self, idx: AttributePlace, llfn: &Value) { let mut regular = self.regular; unsafe { let deref = self.pointee_size.bytes(); @@ -61,9 +61,6 @@ impl ArgAttributesExt for ArgAttributes { if let Some(align) = self.pointee_align { llvm::LLVMRustAddAlignmentAttr(llfn, idx.as_uint(), align.bytes() as u32); } - if regular.contains(ArgAttribute::ByVal) { - llvm::LLVMRustAddByValAttr(llfn, idx.as_uint(), ty.unwrap()); - } regular.for_each_kind(|attr| attr.apply_llfn(idx, llfn)); match self.arg_ext { ArgExtension::None => {} @@ -77,7 +74,7 @@ impl ArgAttributesExt for ArgAttributes { } } - fn apply_callsite(&self, idx: AttributePlace, callsite: &Value, ty: Option<&Type>) { + fn apply_attrs_to_callsite(&self, idx: AttributePlace, callsite: &Value) { let mut regular = self.regular; unsafe { let deref = self.pointee_size.bytes(); @@ -100,9 +97,6 @@ impl ArgAttributesExt for ArgAttributes { align.bytes() as u32, ); } - if regular.contains(ArgAttribute::ByVal) { - llvm::LLVMRustAddByValCallSiteAttr(callsite, idx.as_uint(), ty.unwrap()); - } regular.for_each_kind(|attr| attr.apply_callsite(idx, callsite)); match self.arg_ext { ArgExtension::None => {} @@ -285,10 +279,12 @@ impl ArgAbiExt<'ll, 'tcx> for ArgAbi<'tcx, Ty<'tcx>> { PassMode::Pair(..) => { OperandValue::Pair(next(), next()).store(bx, dst); } - PassMode::Indirect(_, Some(_)) => { + PassMode::Indirect { attrs: _, extra_attrs: Some(_), on_stack: _ } => { OperandValue::Ref(next(), Some(next()), self.layout.align.abi).store(bx, dst); } - PassMode::Direct(_) | PassMode::Indirect(_, None) | PassMode::Cast(_) => { + PassMode::Direct(_) + | PassMode::Indirect { attrs: _, extra_attrs: None, on_stack: _ } + | PassMode::Cast(_) => { let next_arg = next(); self.store(bx, next_arg, dst); } @@ -333,14 +329,14 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { if let PassMode::Pair(_, _) = arg.mode { 2 } else { 1 } ).sum(); let mut llargument_tys = Vec::with_capacity( - if let PassMode::Indirect(..) = self.ret.mode { 1 } else { 0 } + args_capacity, + if let PassMode::Indirect { .. } = self.ret.mode { 1 } else { 0 } + args_capacity, ); let llreturn_ty = match self.ret.mode { PassMode::Ignore => cx.type_void(), PassMode::Direct(_) | PassMode::Pair(..) => self.ret.layout.immediate_llvm_type(cx), PassMode::Cast(cast) => cast.llvm_type(cx), - PassMode::Indirect(..) => { + PassMode::Indirect { .. } => { llargument_tys.push(cx.type_ptr_to(self.ret.memory_ty(cx))); cx.type_void() } @@ -360,7 +356,7 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 1, true)); continue; } - PassMode::Indirect(_, Some(_)) => { + PassMode::Indirect { attrs: _, extra_attrs: Some(_), on_stack: _ } => { let ptr_ty = cx.tcx.mk_mut_ptr(arg.layout.ty); let ptr_layout = cx.layout_of(ptr_ty); llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 0, true)); @@ -368,7 +364,9 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { continue; } PassMode::Cast(cast) => cast.llvm_type(cx), - PassMode::Indirect(_, None) => cx.type_ptr_to(arg.memory_ty(cx)), + PassMode::Indirect { attrs: _, extra_attrs: None, on_stack: _ } => { + cx.type_ptr_to(arg.memory_ty(cx)) + } }; llargument_tys.push(llarg_ty); } @@ -420,35 +418,53 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { } let mut i = 0; - let mut apply = |attrs: &ArgAttributes, ty: Option<&Type>| { - attrs.apply_llfn(llvm::AttributePlace::Argument(i), llfn, ty); + let mut apply = |attrs: &ArgAttributes| { + attrs.apply_attrs_to_llfn(llvm::AttributePlace::Argument(i), llfn); i += 1; + i - 1 }; match self.ret.mode { PassMode::Direct(ref attrs) => { - attrs.apply_llfn(llvm::AttributePlace::ReturnValue, llfn, None); + attrs.apply_attrs_to_llfn(llvm::AttributePlace::ReturnValue, llfn); + } + PassMode::Indirect { ref attrs, extra_attrs: _, on_stack } => { + assert!(!on_stack); + apply(attrs); } - PassMode::Indirect(ref attrs, _) => apply(attrs, Some(self.ret.layout.llvm_type(cx))), _ => {} } for arg in &self.args { if arg.pad.is_some() { - apply(&ArgAttributes::new(), None); + apply(&ArgAttributes::new()); } match arg.mode { PassMode::Ignore => {} - PassMode::Direct(ref attrs) | PassMode::Indirect(ref attrs, None) => { - apply(attrs, Some(arg.layout.llvm_type(cx))) + PassMode::Indirect { ref attrs, extra_attrs: None, on_stack: true } => { + let i = apply(attrs); + unsafe { + llvm::LLVMRustAddByValAttr( + llfn, + llvm::AttributePlace::Argument(i).as_uint(), + arg.layout.llvm_type(cx), + ); + } + } + PassMode::Direct(ref attrs) + | PassMode::Indirect { ref attrs, extra_attrs: None, on_stack: false } => { + apply(attrs); } - PassMode::Indirect(ref attrs, Some(ref extra_attrs)) => { - apply(attrs, None); - apply(extra_attrs, None); + PassMode::Indirect { ref attrs, extra_attrs: Some(ref extra_attrs), on_stack } => { + assert!(!on_stack); + apply(attrs); + apply(extra_attrs); } PassMode::Pair(ref a, ref b) => { - apply(a, None); - apply(b, None); + apply(a); + apply(b); + } + PassMode::Cast(_) => { + apply(&ArgAttributes::new()); } - PassMode::Cast(_) => apply(&ArgAttributes::new(), None), } } } @@ -457,15 +473,19 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { // FIXME(wesleywiser, eddyb): We should apply `nounwind` and `noreturn` as appropriate to this callsite. let mut i = 0; - let mut apply = |attrs: &ArgAttributes, ty: Option<&Type>| { - attrs.apply_callsite(llvm::AttributePlace::Argument(i), callsite, ty); + let mut apply = |attrs: &ArgAttributes| { + attrs.apply_attrs_to_callsite(llvm::AttributePlace::Argument(i), callsite); i += 1; + i - 1 }; match self.ret.mode { PassMode::Direct(ref attrs) => { - attrs.apply_callsite(llvm::AttributePlace::ReturnValue, callsite, None); + attrs.apply_attrs_to_callsite(llvm::AttributePlace::ReturnValue, callsite); + } + PassMode::Indirect { ref attrs, extra_attrs: _, on_stack } => { + assert!(!on_stack); + apply(attrs); } - PassMode::Indirect(ref attrs, _) => apply(attrs, Some(self.ret.layout.llvm_type(bx))), _ => {} } if let abi::Abi::Scalar(ref scalar) = self.ret.layout.abi { @@ -483,22 +503,39 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { } for arg in &self.args { if arg.pad.is_some() { - apply(&ArgAttributes::new(), None); + apply(&ArgAttributes::new()); } match arg.mode { PassMode::Ignore => {} - PassMode::Direct(ref attrs) | PassMode::Indirect(ref attrs, None) => { - apply(attrs, Some(arg.layout.llvm_type(bx))) + PassMode::Indirect { ref attrs, extra_attrs: None, on_stack: true } => { + let i = apply(attrs); + unsafe { + llvm::LLVMRustAddByValCallSiteAttr( + callsite, + llvm::AttributePlace::Argument(i).as_uint(), + arg.layout.llvm_type(bx), + ); + } } - PassMode::Indirect(ref attrs, Some(ref extra_attrs)) => { - apply(attrs, None); - apply(extra_attrs, None); + PassMode::Direct(ref attrs) + | PassMode::Indirect { ref attrs, extra_attrs: None, on_stack: false } => { + apply(attrs); + } + PassMode::Indirect { + ref attrs, + extra_attrs: Some(ref extra_attrs), + on_stack: _, + } => { + apply(attrs); + apply(extra_attrs); } PassMode::Pair(ref a, ref b) => { - apply(a, None); - apply(b, None); + apply(a); + apply(b); + } + PassMode::Cast(_) => { + apply(&ArgAttributes::new()); } - PassMode::Cast(_) => apply(&ArgAttributes::new(), None), } } diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index ee40095883ded..fd20709f5d80f 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -255,7 +255,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { return; } let llval = match self.fn_abi.ret.mode { - PassMode::Ignore | PassMode::Indirect(..) => { + PassMode::Ignore | PassMode::Indirect { .. } => { bx.ret_void(); return; } @@ -1101,7 +1101,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // Force by-ref if we have to load through a cast pointer. let (mut llval, align, by_ref) = match op.val { Immediate(_) | Pair(..) => match arg.mode { - PassMode::Indirect(..) | PassMode::Cast(_) => { + PassMode::Indirect { .. } | PassMode::Cast(_) => { let scratch = PlaceRef::alloca(bx, arg.layout); op.val.store(bx, scratch); (scratch.llval, scratch.align, true) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 32bd9651ece99..24d2ee6ec34a7 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2801,7 +2801,8 @@ where for arg in &mut self.args { fixup(arg, false); } - if let PassMode::Indirect(ref mut attrs, _) = self.ret.mode { + if let PassMode::Indirect { ref mut attrs, extra_attrs: _, on_stack: _ } = self.ret.mode + { attrs.set(ArgAttribute::StructRet); } return; diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 9d657250c7c79..0931e8a37714d 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -36,9 +36,12 @@ pub enum PassMode { /// a single uniform or a pair of registers. Cast(CastTarget), /// Pass the argument indirectly via a hidden pointer. - /// The second value, if any, is for the extra data (vtable or length) + /// The `extra_attrs` value, if any, is for the extra data (vtable or length) /// which indicates that it refers to an unsized rvalue. - Indirect(ArgAttributes, Option), + /// `on_stack` defines that the the value should be passed at a fixed + /// stack offset in accordance to the ABI rather than passed using a + /// pointer. This corresponds to the `byval` LLVM argument attribute. + Indirect { attrs: ArgAttributes, extra_attrs: Option, on_stack: bool }, } // Hack to disable non_upper_case_globals only for the bitflags! and not for the rest @@ -52,7 +55,6 @@ mod attr_impl { bitflags::bitflags! { #[derive(Default)] pub struct ArgAttribute: u16 { - const ByVal = 1 << 0; const NoAlias = 1 << 1; const NoCapture = 1 << 2; const NonNull = 1 << 3; @@ -460,14 +462,14 @@ impl<'a, Ty> ArgAbi<'a, Ty> { let extra_attrs = self.layout.is_unsized().then_some(ArgAttributes::new()); - self.mode = PassMode::Indirect(attrs, extra_attrs); + self.mode = PassMode::Indirect { attrs, extra_attrs, on_stack: false }; } pub fn make_indirect_byval(&mut self) { self.make_indirect(); match self.mode { - PassMode::Indirect(ref mut attrs, _) => { - attrs.set(ArgAttribute::ByVal); + PassMode::Indirect { attrs: _, extra_attrs: _, ref mut on_stack } => { + *on_stack = true; } _ => unreachable!(), } @@ -500,15 +502,15 @@ impl<'a, Ty> ArgAbi<'a, Ty> { } pub fn is_indirect(&self) -> bool { - matches!(self.mode, PassMode::Indirect(..)) + matches!(self.mode, PassMode::Indirect {..}) } pub fn is_sized_indirect(&self) -> bool { - matches!(self.mode, PassMode::Indirect(_, None)) + matches!(self.mode, PassMode::Indirect { attrs: _, extra_attrs: None, on_stack: _ }) } pub fn is_unsized_indirect(&self) -> bool { - matches!(self.mode, PassMode::Indirect(_, Some(_))) + matches!(self.mode, PassMode::Indirect { attrs: _, extra_attrs: Some(_), on_stack: _ }) } pub fn is_ignore(&self) -> bool { @@ -617,7 +619,7 @@ impl<'a, Ty> FnAbi<'a, Ty> { a => return Err(format!("unrecognized arch \"{}\" in target specification", a)), } - if let PassMode::Indirect(ref mut attrs, _) = self.ret.mode { + if let PassMode::Indirect { ref mut attrs, extra_attrs: _, on_stack: _ } = self.ret.mode { attrs.set(ArgAttribute::StructRet); } diff --git a/compiler/rustc_target/src/abi/call/x86.rs b/compiler/rustc_target/src/abi/call/x86.rs index 07bf1e94c617e..713b4100a3335 100644 --- a/compiler/rustc_target/src/abi/call/x86.rs +++ b/compiler/rustc_target/src/abi/call/x86.rs @@ -92,9 +92,14 @@ where for arg in &mut fn_abi.args { let attrs = match arg.mode { - PassMode::Ignore | PassMode::Indirect(_, None) => continue, + PassMode::Ignore + | PassMode::Indirect { attrs: _, extra_attrs: None, on_stack: _ } => { + continue; + } PassMode::Direct(ref mut attrs) => attrs, - PassMode::Pair(..) | PassMode::Indirect(_, Some(_)) | PassMode::Cast(_) => { + PassMode::Pair(..) + | PassMode::Indirect { attrs: _, extra_attrs: Some(_), on_stack: _ } + | PassMode::Cast(_) => { unreachable!("x86 shouldn't be passing arguments by {:?}", arg.mode) } }; From 39b8b2b623087fa643daeed424bd2fa79076f463 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sat, 14 Nov 2020 19:16:43 +0100 Subject: [PATCH 3/5] Remove StructRet arg attr It is applied exactly when the return value has an indirect pass mode. Except for InReg on x86 fastcall, arg attrs are now only used for optimization purposes and thus are fine to ignore. --- compiler/rustc_codegen_llvm/src/abi.rs | 9 ++++++--- compiler/rustc_middle/src/ty/layout.rs | 4 ---- compiler/rustc_target/src/abi/call/mod.rs | 5 ----- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index e988fdb9d5c53..bb152104d64d3 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -36,7 +36,7 @@ impl ArgAttributeExt for ArgAttribute { where F: FnMut(llvm::Attribute), { - for_each_kind!(self, f, NoAlias, NoCapture, NonNull, ReadOnly, StructRet, InReg) + for_each_kind!(self, f, NoAlias, NoCapture, NonNull, ReadOnly, InReg) } } @@ -429,7 +429,8 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { } PassMode::Indirect { ref attrs, extra_attrs: _, on_stack } => { assert!(!on_stack); - apply(attrs); + let i = apply(attrs); + llvm::Attribute::StructRet.apply_llfn(llvm::AttributePlace::Argument(i), llfn); } _ => {} } @@ -484,7 +485,9 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { } PassMode::Indirect { ref attrs, extra_attrs: _, on_stack } => { assert!(!on_stack); - apply(attrs); + let i = apply(attrs); + llvm::Attribute::StructRet + .apply_callsite(llvm::AttributePlace::Argument(i), callsite); } _ => {} } diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 24d2ee6ec34a7..666160ba49763 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2801,10 +2801,6 @@ where for arg in &mut self.args { fixup(arg, false); } - if let PassMode::Indirect { ref mut attrs, extra_attrs: _, on_stack: _ } = self.ret.mode - { - attrs.set(ArgAttribute::StructRet); - } return; } diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 0931e8a37714d..91b004c41d956 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -59,7 +59,6 @@ mod attr_impl { const NoCapture = 1 << 2; const NonNull = 1 << 3; const ReadOnly = 1 << 4; - const StructRet = 1 << 6; const InReg = 1 << 8; } } @@ -619,10 +618,6 @@ impl<'a, Ty> FnAbi<'a, Ty> { a => return Err(format!("unrecognized arch \"{}\" in target specification", a)), } - if let PassMode::Indirect { ref mut attrs, extra_attrs: _, on_stack: _ } = self.ret.mode { - attrs.set(ArgAttribute::StructRet); - } - Ok(()) } } From 6a5f537fb902864ad3b3b4a3b043bcc0ee64ec54 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sun, 15 Nov 2020 10:12:10 +0100 Subject: [PATCH 4/5] Rename prefix_chunk to prefix_chunk_size --- compiler/rustc_codegen_llvm/src/abi.rs | 2 +- compiler/rustc_target/src/abi/call/mips64.rs | 2 +- compiler/rustc_target/src/abi/call/mod.rs | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index bb152104d64d3..915dd3d9eda15 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -158,7 +158,7 @@ impl LlvmType for CastTarget { .prefix .iter() .flat_map(|option_kind| { - option_kind.map(|kind| Reg { kind, size: self.prefix_chunk }.llvm_type(cx)) + option_kind.map(|kind| Reg { kind, size: self.prefix_chunk_size }.llvm_type(cx)) }) .chain((0..rest_count).map(|_| rest_ll_unit)) .collect(); diff --git a/compiler/rustc_target/src/abi/call/mips64.rs b/compiler/rustc_target/src/abi/call/mips64.rs index 739ec6112ad3b..f91552740feb8 100644 --- a/compiler/rustc_target/src/abi/call/mips64.rs +++ b/compiler/rustc_target/src/abi/call/mips64.rs @@ -137,7 +137,7 @@ where let rest_size = size - Size::from_bytes(8) * prefix_index as u64; arg.cast_to(CastTarget { prefix, - prefix_chunk: Size::from_bytes(8), + prefix_chunk_size: Size::from_bytes(8), rest: Uniform { unit: Reg::i64(), total: rest_size }, }); } diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 91b004c41d956..83e08f637e3a2 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -203,7 +203,7 @@ impl Uniform { #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct CastTarget { pub prefix: [Option; 8], - pub prefix_chunk: Size, + pub prefix_chunk_size: Size, pub rest: Uniform, } @@ -215,7 +215,7 @@ impl From for CastTarget { impl From for CastTarget { fn from(uniform: Uniform) -> CastTarget { - CastTarget { prefix: [None; 8], prefix_chunk: Size::ZERO, rest: uniform } + CastTarget { prefix: [None; 8], prefix_chunk_size: Size::ZERO, rest: uniform } } } @@ -223,13 +223,13 @@ impl CastTarget { pub fn pair(a: Reg, b: Reg) -> CastTarget { CastTarget { prefix: [Some(a.kind), None, None, None, None, None, None, None], - prefix_chunk: a.size, + prefix_chunk_size: a.size, rest: Uniform::from(b), } } pub fn size(&self, cx: &C) -> Size { - (self.prefix_chunk * self.prefix.iter().filter(|x| x.is_some()).count() as u64) + (self.prefix_chunk_size * self.prefix.iter().filter(|x| x.is_some()).count() as u64) .align_to(self.rest.align(cx)) + self.rest.total } @@ -237,7 +237,7 @@ impl CastTarget { pub fn align(&self, cx: &C) -> Align { self.prefix .iter() - .filter_map(|x| x.map(|kind| Reg { kind, size: self.prefix_chunk }.align(cx))) + .filter_map(|x| x.map(|kind| Reg { kind, size: self.prefix_chunk_size }.align(cx))) .fold(cx.data_layout().aggregate_align.abi.max(self.rest.align(cx)), |acc, align| { acc.max(align) }) From 43968aa8b8c1d0073410d3af36bd83debf6fcf78 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sat, 21 Nov 2020 19:06:35 +0100 Subject: [PATCH 5/5] Replace sext() and zext() with single ext() method --- compiler/rustc_middle/src/ty/layout.rs | 4 ++-- compiler/rustc_target/src/abi/call/mips64.rs | 4 ++-- compiler/rustc_target/src/abi/call/mod.rs | 16 +++++----------- compiler/rustc_target/src/abi/call/riscv.rs | 4 ++-- 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 666160ba49763..fa0711193df48 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -15,7 +15,7 @@ use rustc_session::{DataTypeKind, FieldInfo, SizeKind, VariantInfo}; use rustc_span::symbol::{Ident, Symbol}; use rustc_span::DUMMY_SP; use rustc_target::abi::call::{ - ArgAbi, ArgAttribute, ArgAttributes, Conv, FnAbi, PassMode, Reg, RegKind, + ArgAbi, ArgAttribute, ArgAttributes, ArgExtension, Conv, FnAbi, PassMode, Reg, RegKind, }; use rustc_target::abi::*; use rustc_target::spec::{abi::Abi as SpecAbi, HasTargetSpec, PanicStrategy}; @@ -2619,7 +2619,7 @@ where is_return: bool| { // Booleans are always an i1 that needs to be zero-extended. if scalar.is_bool() { - attrs.zext(); + attrs.ext(ArgExtension::Zext); return; } diff --git a/compiler/rustc_target/src/abi/call/mips64.rs b/compiler/rustc_target/src/abi/call/mips64.rs index f91552740feb8..a630c84142b4d 100644 --- a/compiler/rustc_target/src/abi/call/mips64.rs +++ b/compiler/rustc_target/src/abi/call/mips64.rs @@ -1,4 +1,4 @@ -use crate::abi::call::{ArgAbi, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform}; +use crate::abi::call::{ArgAbi, ArgExtension, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform}; use crate::abi::{self, HasDataLayout, LayoutOf, Size, TyAndLayout, TyAndLayoutMethods}; fn extend_integer_width_mips(arg: &mut ArgAbi<'_, Ty>, bits: u64) { @@ -7,7 +7,7 @@ fn extend_integer_width_mips(arg: &mut ArgAbi<'_, Ty>, bits: u64) { if let abi::Int(i, signed) = scalar.value { if !signed && i.size().bits() == 32 { if let PassMode::Direct(ref mut attrs) = arg.mode { - attrs.sext(); + attrs.ext(ArgExtension::Sext); return; } } diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 83e08f637e3a2..5de9a8dfa7ac1 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -96,15 +96,9 @@ impl ArgAttributes { } } - pub fn zext(&mut self) -> &mut Self { - assert_ne!(self.arg_ext, ArgExtension::Sext); - self.arg_ext = ArgExtension::Zext; - self - } - - pub fn sext(&mut self) -> &mut Self { - assert_ne!(self.arg_ext, ArgExtension::Zext); - self.arg_ext = ArgExtension::Sext; + pub fn ext(&mut self, ext: ArgExtension) -> &mut Self { + assert!(self.arg_ext == ArgExtension::None || self.arg_ext == ext); + self.arg_ext = ext; self } @@ -481,9 +475,9 @@ impl<'a, Ty> ArgAbi<'a, Ty> { if i.size().bits() < bits { if let PassMode::Direct(ref mut attrs) = self.mode { if signed { - attrs.sext() + attrs.ext(ArgExtension::Sext) } else { - attrs.zext() + attrs.ext(ArgExtension::Zext) }; } } diff --git a/compiler/rustc_target/src/abi/call/riscv.rs b/compiler/rustc_target/src/abi/call/riscv.rs index 9c4ac85ce6135..1ab881dd13d1c 100644 --- a/compiler/rustc_target/src/abi/call/riscv.rs +++ b/compiler/rustc_target/src/abi/call/riscv.rs @@ -4,7 +4,7 @@ // Reference: Clang RISC-V ELF psABI lowering code // https://github.com/llvm/llvm-project/blob/8e780252a7284be45cf1ba224cabd884847e8e92/clang/lib/CodeGen/TargetInfo.cpp#L9311-L9773 -use crate::abi::call::{ArgAbi, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform}; +use crate::abi::call::{ArgAbi, ArgExtension, CastTarget, FnAbi, PassMode, Reg, RegKind, Uniform}; use crate::abi::{ self, Abi, FieldsShape, HasDataLayout, LayoutOf, Size, TyAndLayout, TyAndLayoutMethods, }; @@ -308,7 +308,7 @@ fn extend_integer_width<'a, Ty>(arg: &mut ArgAbi<'a, Ty>, xlen: u64) { // 32-bit integers are always sign-extended if i.size().bits() == 32 && xlen > 32 { if let PassMode::Direct(ref mut attrs) = arg.mode { - attrs.sext(); + attrs.ext(ArgExtension::Sext); return; } }