From 096f14d37488ec5f2855cda241da5c795d738429 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 4 Sep 2023 20:29:50 +0200 Subject: [PATCH 1/2] Add support for NonNull function attribute --- src/abi.rs | 54 ++++++++++++++++++++++++++++---------------- src/declare.rs | 6 ++++- src/intrinsic/mod.rs | 2 +- src/type_of.rs | 3 ++- 4 files changed, 43 insertions(+), 22 deletions(-) diff --git a/src/abi.rs b/src/abi.rs index 35bb0b6e5f4e2..a2825773bd390 100644 --- a/src/abi.rs +++ b/src/abi.rs @@ -1,4 +1,4 @@ -use gccjit::{ToLValue, ToRValue, Type}; +use gccjit::{FnAttribute, ToLValue, ToRValue, Type}; use rustc_codegen_ssa::traits::{AbiBuilderMethods, BaseTypeMethods}; use rustc_data_structures::fx::FxHashSet; use rustc_middle::bug; @@ -98,12 +98,12 @@ impl GccType for Reg { pub trait FnAbiGccExt<'gcc, 'tcx> { // TODO(antoyo): return a function pointer type instead? - fn gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> (Type<'gcc>, Vec>, bool, FxHashSet); + fn gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> (Type<'gcc>, Vec>, bool, FxHashSet, Vec>); fn ptr_to_gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> Type<'gcc>; } impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { - fn gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> (Type<'gcc>, Vec>, bool, FxHashSet) { + fn gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> (Type<'gcc>, Vec>, bool, FxHashSet, Vec>) { let mut on_stack_param_indices = FxHashSet::default(); // This capacity calculation is approximate. @@ -121,19 +121,23 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { cx.type_void() } }; + let mut non_null_args = Vec::new(); #[cfg(feature = "master")] - let apply_attrs = |ty: Type<'gcc>, attrs: &ArgAttributes| { - if cx.sess().opts.optimize != config::OptLevel::No - && attrs.regular.contains(rustc_target::abi::call::ArgAttribute::NoAlias) - { - ty.make_restrict() - } else { - ty + let mut apply_attrs = |mut ty: Type<'gcc>, attrs: &ArgAttributes, arg_index: usize| { + if cx.sess().opts.optimize == config::OptLevel::No { + return ty; } + if attrs.regular.contains(rustc_target::abi::call::ArgAttribute::NoAlias) { + ty = ty.make_restrict() + } + if attrs.regular.contains(rustc_target::abi::call::ArgAttribute::NonNull) { + non_null_args.push(arg_index as i32 + 1); + } + ty }; #[cfg(not(feature = "master"))] - let apply_attrs = |ty: Type<'gcc>, _attrs: &ArgAttributes| { + let apply_attrs = |ty: Type<'gcc>, _attrs: &ArgAttributes, _arg_index: usize| { ty }; @@ -141,8 +145,9 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { let arg_ty = match arg.mode { PassMode::Ignore => continue, PassMode::Pair(a, b) => { - argument_tys.push(apply_attrs(arg.layout.scalar_pair_element_gcc_type(cx, 0), &a)); - argument_tys.push(apply_attrs(arg.layout.scalar_pair_element_gcc_type(cx, 1), &b)); + let arg_pos = argument_tys.len(); + argument_tys.push(apply_attrs(arg.layout.scalar_pair_element_gcc_type(cx, 0), &a, arg_pos)); + argument_tys.push(apply_attrs(arg.layout.scalar_pair_element_gcc_type(cx, 1), &b, arg_pos + 1)); continue; } PassMode::Cast { ref cast, pad_i32 } => { @@ -151,30 +156,41 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { argument_tys.push(Reg::i32().gcc_type(cx)); } let ty = cast.gcc_type(cx); - apply_attrs(ty, &cast.attrs) + apply_attrs(ty, &cast.attrs, argument_tys.len()) } PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: true } => { // This is a "byval" argument, so we don't apply the `restrict` attribute on it. on_stack_param_indices.insert(argument_tys.len()); arg.memory_ty(cx) }, - PassMode::Direct(attrs) => apply_attrs(arg.layout.immediate_gcc_type(cx), &attrs), + PassMode::Direct(attrs) => apply_attrs(arg.layout.immediate_gcc_type(cx), &attrs, argument_tys.len()), PassMode::Indirect { attrs, meta_attrs: None, on_stack: false } => { - apply_attrs(cx.type_ptr_to(arg.memory_ty(cx)), &attrs) + apply_attrs(cx.type_ptr_to(arg.memory_ty(cx)), &attrs, argument_tys.len()) } PassMode::Indirect { attrs, meta_attrs: Some(meta_attrs), on_stack } => { assert!(!on_stack); - apply_attrs(apply_attrs(cx.type_ptr_to(arg.memory_ty(cx)), &attrs), &meta_attrs) + let ty = apply_attrs(cx.type_ptr_to(arg.memory_ty(cx)), &attrs, argument_tys.len()); + apply_attrs(ty, &meta_attrs, argument_tys.len()) } }; argument_tys.push(arg_ty); } - (return_ty, argument_tys, self.c_variadic, on_stack_param_indices) + #[cfg(feature = "master")] + let fn_attrs = if non_null_args.is_empty() { + Vec::new() + } else { + vec![FnAttribute::NonNull(non_null_args)] + }; + #[cfg(not(feature = "master"))] + let fn_attrs = Vec::new(); + + (return_ty, argument_tys, self.c_variadic, on_stack_param_indices, fn_attrs) } fn ptr_to_gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> Type<'gcc> { - let (return_type, params, variadic, on_stack_param_indices) = self.gcc_type(cx); + // FIXME: Should we do something with `fn_attrs`? + let (return_type, params, variadic, on_stack_param_indices, _fn_attrs) = self.gcc_type(cx); let pointer_type = cx.context.new_function_pointer_type(None, return_type, ¶ms, variadic); cx.on_stack_params.borrow_mut().insert(pointer_type.dyncast_function_ptr_type().expect("function ptr type"), on_stack_param_indices); pointer_type diff --git a/src/declare.rs b/src/declare.rs index e673d0af4c7ef..409f112ca7397 100644 --- a/src/declare.rs +++ b/src/declare.rs @@ -80,9 +80,13 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { } pub fn declare_fn(&self, name: &str, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> Function<'gcc> { - let (return_type, params, variadic, on_stack_param_indices) = fn_abi.gcc_type(self); + let (return_type, params, variadic, on_stack_param_indices, fn_attrs) = fn_abi.gcc_type(self); let func = declare_raw_fn(self, name, () /*fn_abi.llvm_cconv()*/, return_type, ¶ms, variadic); self.on_stack_function_params.borrow_mut().insert(func, on_stack_param_indices); + // We need to handle `nonnull` here where we still have access to function args. + for fn_attr in fn_attrs { + func.add_attribute(fn_attr); + } func } diff --git a/src/intrinsic/mod.rs b/src/intrinsic/mod.rs index 9caed459a2926..4d0670d802d52 100644 --- a/src/intrinsic/mod.rs +++ b/src/intrinsic/mod.rs @@ -1197,7 +1197,7 @@ fn get_rust_try_fn<'a, 'gcc, 'tcx>(cx: &'a CodegenCx<'gcc, 'tcx>, codegen: &mut #[cfg(feature="master")] fn gen_fn<'a, 'gcc, 'tcx>(cx: &'a CodegenCx<'gcc, 'tcx>, name: &str, rust_fn_sig: ty::PolyFnSig<'tcx>, codegen: &mut dyn FnMut(Builder<'a, 'gcc, 'tcx>)) -> (Type<'gcc>, Function<'gcc>) { let fn_abi = cx.fn_abi_of_fn_ptr(rust_fn_sig, ty::List::empty()); - let (typ, _, _, _) = fn_abi.gcc_type(cx); + let (typ, _, _, _, _) = fn_abi.gcc_type(cx); // FIXME(eddyb) find a nicer way to do this. cx.linkage.set(FunctionType::Internal); let func = cx.declare_fn(name, fn_abi); diff --git a/src/type_of.rs b/src/type_of.rs index c2eab295acd29..4563e32301f50 100644 --- a/src/type_of.rs +++ b/src/type_of.rs @@ -372,7 +372,8 @@ impl<'gcc, 'tcx> LayoutTypeMethods<'tcx> for CodegenCx<'gcc, 'tcx> { } fn fn_decl_backend_type(&self, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> Type<'gcc> { - let (return_type, param_types, variadic, _) = fn_abi.gcc_type(self); + // // FIXME: Should we do something with `fn_attrs`? + let (return_type, param_types, variadic, _, _fn_attrs) = fn_abi.gcc_type(self); self.context.new_function_pointer_type(None, return_type, ¶m_types, variadic) } } From 0348a5f17adbb922cdb2a7b75bfee4045fe1adf8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 17 Oct 2023 21:55:36 +0200 Subject: [PATCH 2/2] Improve code readability --- src/abi.rs | 34 +++++++++++++++++++++++++++------- src/declare.rs | 15 ++++++++++----- src/intrinsic/mod.rs | 4 ++-- src/type_of.rs | 13 +++++++++---- 4 files changed, 48 insertions(+), 18 deletions(-) diff --git a/src/abi.rs b/src/abi.rs index a2825773bd390..5600f1ba8a9df 100644 --- a/src/abi.rs +++ b/src/abi.rs @@ -96,14 +96,22 @@ impl GccType for Reg { } } +pub struct FnAbiGcc<'gcc> { + pub return_type: Type<'gcc>, + pub arguments_type: Vec>, + pub is_c_variadic: bool, + pub on_stack_param_indices: FxHashSet, + pub fn_attributes: Vec>, +} + pub trait FnAbiGccExt<'gcc, 'tcx> { // TODO(antoyo): return a function pointer type instead? - fn gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> (Type<'gcc>, Vec>, bool, FxHashSet, Vec>); + fn gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> FnAbiGcc<'gcc>; fn ptr_to_gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> Type<'gcc>; } impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { - fn gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> (Type<'gcc>, Vec>, bool, FxHashSet, Vec>) { + fn gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> FnAbiGcc<'gcc> { let mut on_stack_param_indices = FxHashSet::default(); // This capacity calculation is approximate. @@ -111,7 +119,7 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { self.args.len() + if let PassMode::Indirect { .. } = self.ret.mode { 1 } else { 0 } ); - let return_ty = + let return_type = match self.ret.mode { PassMode::Ignore => cx.type_void(), PassMode::Direct(_) | PassMode::Pair(..) => self.ret.layout.immediate_gcc_type(cx), @@ -185,13 +193,25 @@ impl<'gcc, 'tcx> FnAbiGccExt<'gcc, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { #[cfg(not(feature = "master"))] let fn_attrs = Vec::new(); - (return_ty, argument_tys, self.c_variadic, on_stack_param_indices, fn_attrs) + FnAbiGcc { + return_type, + arguments_type: argument_tys, + is_c_variadic: self.c_variadic, + on_stack_param_indices, + fn_attributes: fn_attrs, + } } fn ptr_to_gcc_type(&self, cx: &CodegenCx<'gcc, 'tcx>) -> Type<'gcc> { - // FIXME: Should we do something with `fn_attrs`? - let (return_type, params, variadic, on_stack_param_indices, _fn_attrs) = self.gcc_type(cx); - let pointer_type = cx.context.new_function_pointer_type(None, return_type, ¶ms, variadic); + // FIXME(antoyo): Should we do something with `FnAbiGcc::fn_attributes`? + let FnAbiGcc { + return_type, + arguments_type, + is_c_variadic, + on_stack_param_indices, + .. + } = self.gcc_type(cx); + let pointer_type = cx.context.new_function_pointer_type(None, return_type, &arguments_type, is_c_variadic); cx.on_stack_params.borrow_mut().insert(pointer_type.dyncast_function_ptr_type().expect("function ptr type"), on_stack_param_indices); pointer_type } diff --git a/src/declare.rs b/src/declare.rs index 409f112ca7397..0b583c074dd23 100644 --- a/src/declare.rs +++ b/src/declare.rs @@ -6,7 +6,7 @@ use rustc_middle::ty::Ty; use rustc_span::Symbol; use rustc_target::abi::call::FnAbi; -use crate::abi::FnAbiGccExt; +use crate::abi::{FnAbiGcc, FnAbiGccExt}; use crate::context::CodegenCx; use crate::intrinsic::llvm; @@ -80,11 +80,16 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { } pub fn declare_fn(&self, name: &str, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> Function<'gcc> { - let (return_type, params, variadic, on_stack_param_indices, fn_attrs) = fn_abi.gcc_type(self); - let func = declare_raw_fn(self, name, () /*fn_abi.llvm_cconv()*/, return_type, ¶ms, variadic); + let FnAbiGcc { + return_type, + arguments_type, + is_c_variadic, + on_stack_param_indices, + fn_attributes, + } = fn_abi.gcc_type(self); + let func = declare_raw_fn(self, name, () /*fn_abi.llvm_cconv()*/, return_type, &arguments_type, is_c_variadic); self.on_stack_function_params.borrow_mut().insert(func, on_stack_param_indices); - // We need to handle `nonnull` here where we still have access to function args. - for fn_attr in fn_attrs { + for fn_attr in fn_attributes { func.add_attribute(fn_attr); } func diff --git a/src/intrinsic/mod.rs b/src/intrinsic/mod.rs index 4d0670d802d52..eaee1d453c53c 100644 --- a/src/intrinsic/mod.rs +++ b/src/intrinsic/mod.rs @@ -1197,7 +1197,7 @@ fn get_rust_try_fn<'a, 'gcc, 'tcx>(cx: &'a CodegenCx<'gcc, 'tcx>, codegen: &mut #[cfg(feature="master")] fn gen_fn<'a, 'gcc, 'tcx>(cx: &'a CodegenCx<'gcc, 'tcx>, name: &str, rust_fn_sig: ty::PolyFnSig<'tcx>, codegen: &mut dyn FnMut(Builder<'a, 'gcc, 'tcx>)) -> (Type<'gcc>, Function<'gcc>) { let fn_abi = cx.fn_abi_of_fn_ptr(rust_fn_sig, ty::List::empty()); - let (typ, _, _, _, _) = fn_abi.gcc_type(cx); + let return_type = fn_abi.gcc_type(cx).return_type; // FIXME(eddyb) find a nicer way to do this. cx.linkage.set(FunctionType::Internal); let func = cx.declare_fn(name, fn_abi); @@ -1207,5 +1207,5 @@ fn gen_fn<'a, 'gcc, 'tcx>(cx: &'a CodegenCx<'gcc, 'tcx>, name: &str, rust_fn_sig let block = Builder::append_block(cx, func_val, "entry-block"); let bx = Builder::build(cx, block); codegen(bx); - (typ, func) + (return_type, func) } diff --git a/src/type_of.rs b/src/type_of.rs index 4563e32301f50..1189e96e3087b 100644 --- a/src/type_of.rs +++ b/src/type_of.rs @@ -9,7 +9,7 @@ use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_target::abi::{self, Abi, Align, F32, F64, FieldsShape, Int, Integer, Pointer, PointeeInfo, Size, TyAbiInterface, Variants}; use rustc_target::abi::call::{CastTarget, FnAbi, Reg}; -use crate::abi::{FnAbiGccExt, GccType}; +use crate::abi::{FnAbiGcc, FnAbiGccExt, GccType}; use crate::context::CodegenCx; use crate::type_::struct_fields; @@ -372,8 +372,13 @@ impl<'gcc, 'tcx> LayoutTypeMethods<'tcx> for CodegenCx<'gcc, 'tcx> { } fn fn_decl_backend_type(&self, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> Type<'gcc> { - // // FIXME: Should we do something with `fn_attrs`? - let (return_type, param_types, variadic, _, _fn_attrs) = fn_abi.gcc_type(self); - self.context.new_function_pointer_type(None, return_type, ¶m_types, variadic) + // FIXME(antoyo): Should we do something with `FnAbiGcc::fn_attributes`? + let FnAbiGcc { + return_type, + arguments_type, + is_c_variadic, + .. + } = fn_abi.gcc_type(self); + self.context.new_function_pointer_type(None, return_type, &arguments_type, is_c_variadic) } }