diff mbox series

[3/5] arm: [MVE intrinsics] Add load_extending and store_truncating function bases

Message ID 20240916093819.12740-4-christophe.lyon@arm.com
State New
Headers show
Series arm, MVE: Refactor the vst and vld intrinsics | expand

Commit Message

Christophe Lyon Sept. 16, 2024, 9:38 a.m. UTC
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(+)

Comments

Richard Earnshaw (lists) Oct. 15, 2024, 2:14 p.m. UTC | #1
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 mbox series

Patch

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"