From 93191f66dc4f4b5d11cb695277be13d38f3248d3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 25 Aug 2022 17:52:37 +1000 Subject: [PATCH 1/5] Box `CastTarget` within `PassMode`. Because `PassMode::Cast` is by far the largest variant, but is relatively rare. This requires making `PassMode` not impl `Copy`, and `Clone` is no longer necessary. This causes lots of sigil adjusting, but nothing very notable. --- src/abi.rs | 4 ++-- src/intrinsic/mod.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/abi.rs b/src/abi.rs index 0ed3e1fbe93f..9b55db6a5476 100644 --- a/src/abi.rs +++ b/src/abi.rs @@ -133,7 +133,7 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { match self.ret.mode { PassMode::Ignore => cx.type_void(), PassMode::Direct(_) | PassMode::Pair(..) => self.ret.layout.immediate_gcc_type(cx), - PassMode::Cast(cast) => cast.gcc_type(cx), + PassMode::Cast(ref cast) => cast.gcc_type(cx), PassMode::Indirect { .. } => { argument_tys.push(cx.type_ptr_to(self.ret.memory_ty(cx))); cx.type_void() @@ -157,7 +157,7 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { PassMode::Indirect { extra_attrs: Some(_), .. } => { unimplemented!(); } - PassMode::Cast(cast) => cast.gcc_type(cx), + PassMode::Cast(ref cast) => cast.gcc_type(cx), PassMode::Indirect { extra_attrs: None, on_stack: true, .. } => { on_stack_param_indices.insert(argument_tys.len()); arg.memory_ty(cx) diff --git a/src/intrinsic/mod.rs b/src/intrinsic/mod.rs index 5fbdedac0c45..f4493776ea26 100644 --- a/src/intrinsic/mod.rs +++ b/src/intrinsic/mod.rs @@ -130,7 +130,7 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { sym::volatile_load | sym::unaligned_volatile_load => { let tp_ty = substs.type_at(0); let mut ptr = args[0].immediate(); - if let PassMode::Cast(ty) = fn_abi.ret.mode { + if let PassMode::Cast(ty) = &fn_abi.ret.mode { ptr = self.pointercast(ptr, self.type_ptr_to(ty.gcc_type(self))); } let load = self.volatile_load(ptr.get_type(), ptr); @@ -320,7 +320,7 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { }; if !fn_abi.ret.is_ignore() { - if let PassMode::Cast(ty) = fn_abi.ret.mode { + if let PassMode::Cast(ty) = &fn_abi.ret.mode { let ptr_llty = self.type_ptr_to(ty.gcc_type(self)); let ptr = self.pointercast(result.llval, ptr_llty); self.store(llval, ptr, result.align); @@ -416,7 +416,7 @@ impl<'gcc, 'tcx> ArgAbiExt<'gcc, 'tcx> for ArgAbi<'tcx, Ty<'tcx>> { else if self.is_unsized_indirect() { bug!("unsized `ArgAbi` must be handled through `store_fn_arg`"); } - else if let PassMode::Cast(cast) = self.mode { + else if let PassMode::Cast(ref cast) = self.mode { // FIXME(eddyb): Figure out when the simpler Store is safe, clang // uses it for i16 -> {i8, i8}, but not for i24 -> {i8, i8, i8}. let can_store_through_cast_ptr = false; From f78592f1bcfba609cc83361dc39c795729292eb1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 25 Aug 2022 19:08:04 +1000 Subject: [PATCH 2/5] Change `FnAbi::args` to a boxed slice. --- src/abi.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/abi.rs b/src/abi.rs index 9b55db6a5476..7f313583c82c 100644 --- a/src/abi.rs +++ b/src/abi.rs @@ -140,7 +140,7 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { } }; - for arg in &self.args { + for arg in self.args.iter() { // add padding if let Some(ty) = arg.pad { argument_tys.push(ty.gcc_type(cx)); From 5a6992916e8957927260171f8802b0d8b8809ec5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 26 Aug 2022 10:37:51 +1000 Subject: [PATCH 3/5] Simplify arg capacity calculations. Currently they try to be very precise. But they are wrong, i.e. they don't match what's happening in the loop below. This code isn't hot enough for it to matter that much. --- src/abi.rs | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/src/abi.rs b/src/abi.rs index 7f313583c82c..87b730d29cdf 100644 --- a/src/abi.rs +++ b/src/abi.rs @@ -107,26 +107,10 @@ pub trait FnAbiGccExt<'gcc, 'tcx> { impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { fn gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> (Type<'gcc>, Vec>, bool, FxHashSet) { let mut on_stack_param_indices = FxHashSet::default(); - let args_capacity: usize = self.args.iter().map(|arg| - if arg.pad.is_some() { - 1 - } - else { - 0 - } + - if let PassMode::Pair(_, _) = arg.mode { - 2 - } else { - 1 - } - ).sum(); + + // This capacity calculation is approximate. let mut argument_tys = Vec::with_capacity( - if let PassMode::Indirect { .. } = self.ret.mode { - 1 - } - else { - 0 - } + args_capacity, + self.args.len() + if let PassMode::Indirect { .. } = self.ret.mode { 1 } else { 0 } ); let return_ty = From d83636d3fac0c0c6b2498bd0699ad6848032f083 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 25 Aug 2022 19:18:01 +1000 Subject: [PATCH 4/5] Turn `ArgAbi::pad` into a `bool`. Because it's only ever set to `None` or `Some(Reg::i32())`. --- src/abi.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/abi.rs b/src/abi.rs index 87b730d29cdf..3186b363e359 100644 --- a/src/abi.rs +++ b/src/abi.rs @@ -126,8 +126,8 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { for arg in self.args.iter() { // add padding - if let Some(ty) = arg.pad { - argument_tys.push(ty.gcc_type(cx)); + if arg.pad_i32 { + argument_tys.push(Reg::i32().gcc_type(cx)); } let arg_ty = match arg.mode { From 89003418b391d0f260b2354472a9b788b8128b7b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 25 Aug 2022 22:19:38 +1000 Subject: [PATCH 5/5] Move `ArgAbi::pad_i32` into `PassMode::Cast`. Because it's only needed for that variant. This shrinks the types and clarifies the logic. --- src/abi.rs | 15 ++++++++------- src/intrinsic/mod.rs | 8 ++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/abi.rs b/src/abi.rs index 3186b363e359..848c34211ff6 100644 --- a/src/abi.rs +++ b/src/abi.rs @@ -117,7 +117,7 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { match self.ret.mode { PassMode::Ignore => cx.type_void(), PassMode::Direct(_) | PassMode::Pair(..) => self.ret.layout.immediate_gcc_type(cx), - PassMode::Cast(ref cast) => cast.gcc_type(cx), + PassMode::Cast(ref cast, _) => cast.gcc_type(cx), PassMode::Indirect { .. } => { argument_tys.push(cx.type_ptr_to(self.ret.memory_ty(cx))); cx.type_void() @@ -125,11 +125,6 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { }; for arg in self.args.iter() { - // add padding - if arg.pad_i32 { - argument_tys.push(Reg::i32().gcc_type(cx)); - } - let arg_ty = match arg.mode { PassMode::Ignore => continue, PassMode::Direct(_) => arg.layout.immediate_gcc_type(cx), @@ -141,7 +136,13 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { PassMode::Indirect { extra_attrs: Some(_), .. } => { unimplemented!(); } - PassMode::Cast(ref cast) => cast.gcc_type(cx), + PassMode::Cast(ref cast, pad_i32) => { + // add padding + if pad_i32 { + argument_tys.push(Reg::i32().gcc_type(cx)); + } + cast.gcc_type(cx) + } PassMode::Indirect { extra_attrs: None, on_stack: true, .. } => { on_stack_param_indices.insert(argument_tys.len()); arg.memory_ty(cx) diff --git a/src/intrinsic/mod.rs b/src/intrinsic/mod.rs index f4493776ea26..352fe7568eff 100644 --- a/src/intrinsic/mod.rs +++ b/src/intrinsic/mod.rs @@ -130,7 +130,7 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { sym::volatile_load | sym::unaligned_volatile_load => { let tp_ty = substs.type_at(0); let mut ptr = args[0].immediate(); - if let PassMode::Cast(ty) = &fn_abi.ret.mode { + if let PassMode::Cast(ty, _) = &fn_abi.ret.mode { ptr = self.pointercast(ptr, self.type_ptr_to(ty.gcc_type(self))); } let load = self.volatile_load(ptr.get_type(), ptr); @@ -320,7 +320,7 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { }; if !fn_abi.ret.is_ignore() { - if let PassMode::Cast(ty) = &fn_abi.ret.mode { + if let PassMode::Cast(ty, _) = &fn_abi.ret.mode { let ptr_llty = self.type_ptr_to(ty.gcc_type(self)); let ptr = self.pointercast(result.llval, ptr_llty); self.store(llval, ptr, result.align); @@ -416,7 +416,7 @@ impl<'gcc, 'tcx> ArgAbiExt<'gcc, 'tcx> for ArgAbi<'tcx, Ty<'tcx>> { else if self.is_unsized_indirect() { bug!("unsized `ArgAbi` must be handled through `store_fn_arg`"); } - else if let PassMode::Cast(ref cast) = self.mode { + else if let PassMode::Cast(ref cast, _) = self.mode { // FIXME(eddyb): Figure out when the simpler Store is safe, clang // uses it for i16 -> {i8, i8}, but not for i24 -> {i8, i8, i8}. let can_store_through_cast_ptr = false; @@ -481,7 +481,7 @@ impl<'gcc, 'tcx> ArgAbiExt<'gcc, 'tcx> for ArgAbi<'tcx, Ty<'tcx>> { PassMode::Indirect { extra_attrs: Some(_), .. } => { OperandValue::Ref(next(), Some(next()), self.layout.align.abi).store(bx, dst); }, - PassMode::Direct(_) | PassMode::Indirect { extra_attrs: None, .. } | PassMode::Cast(_) => { + PassMode::Direct(_) | PassMode::Indirect { extra_attrs: None, .. } | PassMode::Cast(..) => { let next_arg = next(); self.store(bx, next_arg, dst); },