| Message ID | 20240916093819.12740-4-christophe.lyon@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | arm, MVE: Refactor the vst and vld intrinsics | expand |
On 16/09/2024 10:38, Christophe Lyon wrote: > From: Alfie Richards <Alfie.Richards@arm.com> > > This patch adds the load_extending and store_truncating function bases > for MVE intrinsics. > > The constructors have parameters describing the memory element > type/width which is part of the function base name (e.g. "h" in > vldrhq). > > 2024-09-11 Alfie Richards <Alfie.Richards@arm.com> > > gcc/ > > * config/arm/arm-mve-builtins-functions.h > (load_extending): New class. > (store_truncating): New class. > * config/arm/arm-protos.h (arm_mve_data_mode): New helper function. > * config/arm/arm.cc (arm_mve_data_mode): New helper function. This patch is technically ok, but there are some formatting issues that make the code layout slightly confusing and hard to read: + return arm_mve_data_mode (GET_MODE_INNER (mem_mode), + GET_MODE_NUNITS (reg_mode)) + .require (); The stray ".require ();" on its own looks strange given the indentation. Your line is short enough that you can write + return arm_mve_data_mode (GET_MODE_INNER (mem_mode), + GET_MODE_NUNITS (reg_mode)).require (); + unsigned int element_bits = GET_MODE_BITSIZE ( + (fi.type_suffix (0).integer_p + ? m_to_int_mode + : m_to_float_mode.require ())); Here you should put "= GET_MODE_BITSIZE (" on the following line, then indent the arguments to the opening paren of the function arguments: + unsigned int element_bits + = GET_MODE_BITSIZE (fi.type_suffix (0).integer_p + ? m_to_int_mode + : m_to_float_mode.require ()); And you can then lose the extra level of parenthesis. + return arm_mve_data_mode ( + (fi.type_suffix (0).integer_p + ? m_to_int_mode + : m_to_float_mode.require ()), + nunits) + .require (); In this case I'd split the selection operation into a separate statement, giving (if I've got the type correct): + scalar_mode mode = (fi.type_suffix (0).integer_p + ? m_to_int_mode + : m_to_float_mode.require ()); + return arm_mve_data_mode (mode, nunits).require (); OK with those changes. R. > --- > gcc/config/arm/arm-mve-builtins-functions.h | 106 ++++++++++++++++++++ > gcc/config/arm/arm-protos.h | 3 + > gcc/config/arm/arm.cc | 15 +++ > 3 files changed, 124 insertions(+) > > diff --git a/gcc/config/arm/arm-mve-builtins-functions.h b/gcc/config/arm/arm-mve-builtins-functions.h > index ac2a731bff4..e47bc69936e 100644 > --- a/gcc/config/arm/arm-mve-builtins-functions.h > +++ b/gcc/config/arm/arm-mve-builtins-functions.h > @@ -20,6 +20,8 @@ > #ifndef GCC_ARM_MVE_BUILTINS_FUNCTIONS_H > #define GCC_ARM_MVE_BUILTINS_FUNCTIONS_H > > +#include "arm-protos.h" > + > namespace arm_mve { > > /* Wrap T, which is derived from function_base, and indicate that the > @@ -1024,6 +1026,110 @@ public: > } > }; > > +/* A function_base that loads elements from memory and extends them > + to a wider element. The memory element type is a fixed part of > + the function base name. */ > +class load_extending : public function_base > +{ > +public: > + CONSTEXPR load_extending (type_suffix_index signed_memory_type, > + type_suffix_index unsigned_memory_type, > + type_suffix_index float_memory_type) > + : m_signed_memory_type (signed_memory_type), > + m_unsigned_memory_type (unsigned_memory_type), > + m_float_memory_type (float_memory_type) > + {} > + CONSTEXPR load_extending (type_suffix_index signed_memory_type, > + type_suffix_index unsigned_memory_type) > + : m_signed_memory_type (signed_memory_type), > + m_unsigned_memory_type (unsigned_memory_type), > + m_float_memory_type (NUM_TYPE_SUFFIXES) > + {} > + > + unsigned int call_properties (const function_instance &) const override > + { > + return CP_READ_MEMORY; > + } > + > + tree memory_scalar_type (const function_instance &fi) const override > + { > + type_suffix_index memory_type_suffix > + = (fi.type_suffix (0).integer_p > + ? (fi.type_suffix (0).unsigned_p > + ? m_unsigned_memory_type > + : m_signed_memory_type) > + : m_float_memory_type); > + return scalar_types[type_suffixes[memory_type_suffix].vector_type]; > + } > + > + machine_mode memory_vector_mode (const function_instance &fi) const override > + { > + type_suffix_index memory_type_suffix > + = (fi.type_suffix (0).integer_p > + ? (fi.type_suffix (0).unsigned_p > + ? m_unsigned_memory_type > + : m_signed_memory_type) > + : m_float_memory_type); > + machine_mode mem_mode = type_suffixes[memory_type_suffix].vector_mode; > + machine_mode reg_mode = fi.vector_mode (0); > + > + return arm_mve_data_mode (GET_MODE_INNER (mem_mode), > + GET_MODE_NUNITS (reg_mode)) > + .require (); > + } > + > + /* The type of the memory elements. This is part of the function base > + name rather than a true type suffix. */ > + type_suffix_index m_signed_memory_type; > + type_suffix_index m_unsigned_memory_type; > + type_suffix_index m_float_memory_type; > +}; > + > +/* A function_base that truncates vector elements and stores them to memory. > + The memory element width is a fixed part of the function base name. */ > +class store_truncating : public function_base > +{ > +public: > + CONSTEXPR store_truncating (scalar_mode to_int_mode, > + opt_scalar_mode to_float_mode) > + : m_to_int_mode (to_int_mode), m_to_float_mode (to_float_mode) > + {} > + > + unsigned int call_properties (const function_instance &) const override > + { > + return CP_WRITE_MEMORY; > + } > + > + tree memory_scalar_type (const function_instance &fi) const override > + { > + /* In truncating stores, the signedness of the memory element is defined > + to be the same as the signedness of the vector element. The signedness > + doesn't make any difference to the behavior of the function. */ > + type_class_index tclass = fi.type_suffix (0).tclass; > + unsigned int element_bits = GET_MODE_BITSIZE ( > + (fi.type_suffix (0).integer_p > + ? m_to_int_mode > + : m_to_float_mode.require ())); > + type_suffix_index suffix = find_type_suffix (tclass, element_bits); > + return scalar_types[type_suffixes[suffix].vector_type]; > + } > + > + machine_mode memory_vector_mode (const function_instance &fi) const override > + { > + poly_uint64 nunits = GET_MODE_NUNITS (fi.vector_mode (0)); > + return arm_mve_data_mode ( > + (fi.type_suffix (0).integer_p > + ? m_to_int_mode > + : m_to_float_mode.require ()), > + nunits) > + .require (); > + } > + > + /* The mode of a single memory element. */ > + scalar_mode m_to_int_mode; > + opt_scalar_mode m_to_float_mode; > +}; > + > } /* end namespace arm_mve */ > > /* Declare the global function base NAME, creating it from an instance > diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h > index 50cae2b513a..2327f2cfe4e 100644 > --- a/gcc/config/arm/arm-protos.h > +++ b/gcc/config/arm/arm-protos.h > @@ -615,4 +615,7 @@ void arm_initialize_isa (sbitmap, const enum isa_feature *); > const char * arm_gen_far_branch (rtx *, int, const char * , const char *); > > bool arm_mve_immediate_check(rtx, machine_mode, bool); > + > +opt_machine_mode arm_mve_data_mode (scalar_mode, poly_uint64); > + > #endif /* ! GCC_ARM_PROTOS_H */ > diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc > index de34e9867e6..41c4a525613 100644 > --- a/gcc/config/arm/arm.cc > +++ b/gcc/config/arm/arm.cc > @@ -76,6 +76,7 @@ > #include "opts.h" > #include "aarch-common.h" > #include "aarch-common-protos.h" > +#include "machmode.h" > > /* This file should be included last. */ > #include "target-def.h" > @@ -36104,4 +36105,18 @@ arm_output_load_tpidr (rtx dst, bool pred_p) > return ""; > } > > +/* Return the MVE vector mode that has NUNITS elements of mode INNER_MODE. */ > +opt_machine_mode > +arm_mve_data_mode (scalar_mode inner_mode, poly_uint64 nunits) > +{ > + enum mode_class mclass > + = (SCALAR_FLOAT_MODE_P (inner_mode) ? MODE_VECTOR_FLOAT : MODE_VECTOR_INT); > + machine_mode mode; > + FOR_EACH_MODE_IN_CLASS (mode, mclass) > + if (inner_mode == GET_MODE_INNER (mode) > + && known_eq (nunits, GET_MODE_NUNITS (mode))) > + return mode; > + return opt_machine_mode (); > +} > + > #include "gt-arm.h"
diff --git a/gcc/config/arm/arm-mve-builtins-functions.h b/gcc/config/arm/arm-mve-builtins-functions.h index ac2a731bff4..e47bc69936e 100644 --- a/gcc/config/arm/arm-mve-builtins-functions.h +++ b/gcc/config/arm/arm-mve-builtins-functions.h @@ -20,6 +20,8 @@ #ifndef GCC_ARM_MVE_BUILTINS_FUNCTIONS_H #define GCC_ARM_MVE_BUILTINS_FUNCTIONS_H +#include "arm-protos.h" + namespace arm_mve { /* Wrap T, which is derived from function_base, and indicate that the @@ -1024,6 +1026,110 @@ public: } }; +/* A function_base that loads elements from memory and extends them + to a wider element. The memory element type is a fixed part of + the function base name. */ +class load_extending : public function_base +{ +public: + CONSTEXPR load_extending (type_suffix_index signed_memory_type, + type_suffix_index unsigned_memory_type, + type_suffix_index float_memory_type) + : m_signed_memory_type (signed_memory_type), + m_unsigned_memory_type (unsigned_memory_type), + m_float_memory_type (float_memory_type) + {} + CONSTEXPR load_extending (type_suffix_index signed_memory_type, + type_suffix_index unsigned_memory_type) + : m_signed_memory_type (signed_memory_type), + m_unsigned_memory_type (unsigned_memory_type), + m_float_memory_type (NUM_TYPE_SUFFIXES) + {} + + unsigned int call_properties (const function_instance &) const override + { + return CP_READ_MEMORY; + } + + tree memory_scalar_type (const function_instance &fi) const override + { + type_suffix_index memory_type_suffix + = (fi.type_suffix (0).integer_p + ? (fi.type_suffix (0).unsigned_p + ? m_unsigned_memory_type + : m_signed_memory_type) + : m_float_memory_type); + return scalar_types[type_suffixes[memory_type_suffix].vector_type]; + } + + machine_mode memory_vector_mode (const function_instance &fi) const override + { + type_suffix_index memory_type_suffix + = (fi.type_suffix (0).integer_p + ? (fi.type_suffix (0).unsigned_p + ? m_unsigned_memory_type + : m_signed_memory_type) + : m_float_memory_type); + machine_mode mem_mode = type_suffixes[memory_type_suffix].vector_mode; + machine_mode reg_mode = fi.vector_mode (0); + + return arm_mve_data_mode (GET_MODE_INNER (mem_mode), + GET_MODE_NUNITS (reg_mode)) + .require (); + } + + /* The type of the memory elements. This is part of the function base + name rather than a true type suffix. */ + type_suffix_index m_signed_memory_type; + type_suffix_index m_unsigned_memory_type; + type_suffix_index m_float_memory_type; +}; + +/* A function_base that truncates vector elements and stores them to memory. + The memory element width is a fixed part of the function base name. */ +class store_truncating : public function_base +{ +public: + CONSTEXPR store_truncating (scalar_mode to_int_mode, + opt_scalar_mode to_float_mode) + : m_to_int_mode (to_int_mode), m_to_float_mode (to_float_mode) + {} + + unsigned int call_properties (const function_instance &) const override + { + return CP_WRITE_MEMORY; + } + + tree memory_scalar_type (const function_instance &fi) const override + { + /* In truncating stores, the signedness of the memory element is defined + to be the same as the signedness of the vector element. The signedness + doesn't make any difference to the behavior of the function. */ + type_class_index tclass = fi.type_suffix (0).tclass; + unsigned int element_bits = GET_MODE_BITSIZE ( + (fi.type_suffix (0).integer_p + ? m_to_int_mode + : m_to_float_mode.require ())); + type_suffix_index suffix = find_type_suffix (tclass, element_bits); + return scalar_types[type_suffixes[suffix].vector_type]; + } + + machine_mode memory_vector_mode (const function_instance &fi) const override + { + poly_uint64 nunits = GET_MODE_NUNITS (fi.vector_mode (0)); + return arm_mve_data_mode ( + (fi.type_suffix (0).integer_p + ? m_to_int_mode + : m_to_float_mode.require ()), + nunits) + .require (); + } + + /* The mode of a single memory element. */ + scalar_mode m_to_int_mode; + opt_scalar_mode m_to_float_mode; +}; + } /* end namespace arm_mve */ /* Declare the global function base NAME, creating it from an instance diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h index 50cae2b513a..2327f2cfe4e 100644 --- a/gcc/config/arm/arm-protos.h +++ b/gcc/config/arm/arm-protos.h @@ -615,4 +615,7 @@ void arm_initialize_isa (sbitmap, const enum isa_feature *); const char * arm_gen_far_branch (rtx *, int, const char * , const char *); bool arm_mve_immediate_check(rtx, machine_mode, bool); + +opt_machine_mode arm_mve_data_mode (scalar_mode, poly_uint64); + #endif /* ! GCC_ARM_PROTOS_H */ diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc index de34e9867e6..41c4a525613 100644 --- a/gcc/config/arm/arm.cc +++ b/gcc/config/arm/arm.cc @@ -76,6 +76,7 @@ #include "opts.h" #include "aarch-common.h" #include "aarch-common-protos.h" +#include "machmode.h" /* This file should be included last. */ #include "target-def.h" @@ -36104,4 +36105,18 @@ arm_output_load_tpidr (rtx dst, bool pred_p) return ""; } +/* Return the MVE vector mode that has NUNITS elements of mode INNER_MODE. */ +opt_machine_mode +arm_mve_data_mode (scalar_mode inner_mode, poly_uint64 nunits) +{ + enum mode_class mclass + = (SCALAR_FLOAT_MODE_P (inner_mode) ? MODE_VECTOR_FLOAT : MODE_VECTOR_INT); + machine_mode mode; + FOR_EACH_MODE_IN_CLASS (mode, mclass) + if (inner_mode == GET_MODE_INNER (mode) + && known_eq (nunits, GET_MODE_NUNITS (mode))) + return mode; + return opt_machine_mode (); +} + #include "gt-arm.h"
From: Alfie Richards <Alfie.Richards@arm.com> This patch adds the load_extending and store_truncating function bases for MVE intrinsics. The constructors have parameters describing the memory element type/width which is part of the function base name (e.g. "h" in vldrhq). 2024-09-11 Alfie Richards <Alfie.Richards@arm.com> gcc/ * config/arm/arm-mve-builtins-functions.h (load_extending): New class. (store_truncating): New class. * config/arm/arm-protos.h (arm_mve_data_mode): New helper function. * config/arm/arm.cc (arm_mve_data_mode): New helper function. --- gcc/config/arm/arm-mve-builtins-functions.h | 106 ++++++++++++++++++++ gcc/config/arm/arm-protos.h | 3 + gcc/config/arm/arm.cc | 15 +++ 3 files changed, 124 insertions(+)