Re: [PATCH 5/6] aarch64: Implement system register r/w arm ACLE intrinsic functions

[Richard Earnshaw <Richard.Earnshaw@foss.arm.com>]

On 03/10/2023 16:18, Victor Do Nascimento wrote:
> Implement the aarch64 intrinsics for reading and writing system
> registers with the following signatures:
> 
> 	uint32_t __arm_rsr(const char *special_register);
> 	uint64_t __arm_rsr64(const char *special_register);
> 	void* __arm_rsrp(const char *special_register);
> 	float __arm_rsrf(const char *special_register);
> 	double __arm_rsrf64(const char *special_register);
> 	void __arm_wsr(const char *special_register, uint32_t value);
> 	void __arm_wsr64(const char *special_register, uint64_t value);
> 	void __arm_wsrp(const char *special_register, const void *value);
> 	void __arm_wsrf(const char *special_register, float value);
> 	void __arm_wsrf64(const char *special_register, double value);
> 
> gcc/ChangeLog:
> 
> 	* gcc/config/aarch64/aarch64-builtins.cc (enum aarch64_builtins):
> 	Add enums for new builtins.
> 	(aarch64_init_rwsr_builtins): New.
> 	(aarch64_general_init_builtins): Call aarch64_init_rwsr_builtins.
> 	(aarch64_expand_rwsr_builtin):  New.
> 	(aarch64_general_expand_builtin): Call aarch64_general_expand_builtin.
> 	* gcc/config/aarch64/aarch64.md (read_sysregdi): New insn_and_split.
> 	(write_sysregdi): Likewise.
> 	* gcc/config/aarch64/arm_acle.h (__arm_rsr): New.
> 	(__arm_rsrp): Likewise.
> 	(__arm_rsr64): Likewise.
> 	(__arm_rsrf): Likewise.
> 	(__arm_rsrf64): Likewise.
> 	(__arm_wsr): Likewise.
> 	(__arm_wsrp): Likewise.
> 	(__arm_wsr64): Likewise.
> 	(__arm_wsrf): Likewise.
> 	(__arm_wsrf64): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc/testsuite/gcc.target/aarch64/acle/rwsr.c: New.
> 	* gcc/testsuite/gcc.target/aarch64/acle/rwsr-1.c: Likewise.
> ---
>   gcc/config/aarch64/aarch64-builtins.cc        | 200 ++++++++++++++++++
>   gcc/config/aarch64/aarch64.md                 |  17 ++
>   gcc/config/aarch64/arm_acle.h                 |  30 +++
>   .../gcc.target/aarch64/acle/rwsr-1.c          |  20 ++
>   gcc/testsuite/gcc.target/aarch64/acle/rwsr.c  | 144 +++++++++++++
>   5 files changed, 411 insertions(+)
>   create mode 100644 gcc/testsuite/gcc.target/aarch64/acle/rwsr-1.c
>   create mode 100644 gcc/testsuite/gcc.target/aarch64/acle/rwsr.c
> 
> diff --git a/gcc/config/aarch64/aarch64-builtins.cc b/gcc/config/aarch64/aarch64-builtins.cc
> index 04f59fd9a54..d8bb2a989a5 100644
> --- a/gcc/config/aarch64/aarch64-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-builtins.cc
> @@ -808,6 +808,17 @@ enum aarch64_builtins
>     AARCH64_RBIT,
>     AARCH64_RBITL,
>     AARCH64_RBITLL,
> +  /* System register builtins.  */
> +  AARCH64_RSR,
> +  AARCH64_RSRP,
> +  AARCH64_RSR64,
> +  AARCH64_RSRF,
> +  AARCH64_RSRF64,
> +  AARCH64_WSR,
> +  AARCH64_WSRP,
> +  AARCH64_WSR64,
> +  AARCH64_WSRF,
> +  AARCH64_WSRF64,
>     AARCH64_BUILTIN_MAX
>   };
>   
> @@ -1798,6 +1809,65 @@ aarch64_init_rng_builtins (void)
>   				   AARCH64_BUILTIN_RNG_RNDRRS);
>   }
>   
> +/* Add builtins for reading system register.  */
> +static void
> +aarch64_init_rwsr_builtins (void)
> +{
> +  tree fntype = NULL;
> +  tree const_char_ptr_type
> +    = build_pointer_type (build_type_variant (char_type_node, true, false));
> +
> +#define AARCH64_INIT_RWSR_BUILTINS_DECL(F, N, T) \
> +  aarch64_builtin_decls[AARCH64_##F] \
> +    = aarch64_general_add_builtin ("__builtin_aarch64_"#N, T, AARCH64_##F);
> +
> +  fntype
> +    = build_function_type_list (uint32_type_node, const_char_ptr_type, NULL);
> +  AARCH64_INIT_RWSR_BUILTINS_DECL (RSR, rsr, fntype);
> +
> +  fntype
> +    = build_function_type_list (ptr_type_node, const_char_ptr_type, NULL);
> +  AARCH64_INIT_RWSR_BUILTINS_DECL (RSRP, rsrp, fntype);
> +
> +  fntype
> +    = build_function_type_list (uint64_type_node, const_char_ptr_type, NULL);
> +  AARCH64_INIT_RWSR_BUILTINS_DECL (RSR64, rsr64, fntype);
> +
> +  fntype
> +    = build_function_type_list (float_type_node, const_char_ptr_type, NULL);
> +  AARCH64_INIT_RWSR_BUILTINS_DECL (RSRF, rsrf, fntype);
> +
> +  fntype
> +    = build_function_type_list (double_type_node, const_char_ptr_type, NULL);
> +  AARCH64_INIT_RWSR_BUILTINS_DECL (RSRF64, rsrf64, fntype);
> +
> +  fntype
> +    = build_function_type_list (void_type_node, const_char_ptr_type,
> +				uint32_type_node, NULL);
> +
> +  AARCH64_INIT_RWSR_BUILTINS_DECL (WSR, wsr, fntype);
> +
> +  fntype
> +    = build_function_type_list (void_type_node, const_char_ptr_type,
> +				const_ptr_type_node, NULL);
> +  AARCH64_INIT_RWSR_BUILTINS_DECL (WSRP, wsrp, fntype);
> +
> +  fntype
> +    = build_function_type_list (void_type_node, const_char_ptr_type,
> +				uint64_type_node, NULL);
> +  AARCH64_INIT_RWSR_BUILTINS_DECL (WSR64, wsr64, fntype);
> +
> +  fntype
> +    = build_function_type_list (void_type_node, const_char_ptr_type,
> +				float_type_node, NULL);
> +  AARCH64_INIT_RWSR_BUILTINS_DECL (WSRF, wsrf, fntype);
> +
> +  fntype
> +    = build_function_type_list (void_type_node, const_char_ptr_type,
> +				double_type_node, NULL);
> +  AARCH64_INIT_RWSR_BUILTINS_DECL (WSRF64, wsrf64, fntype);
> +}
> +
>   /* Initialize the memory tagging extension (MTE) builtins.  */
>   struct
>   {
> @@ -2019,6 +2089,8 @@ aarch64_general_init_builtins (void)
>     aarch64_init_rng_builtins ();
>     aarch64_init_data_intrinsics ();
>   
> +  aarch64_init_rwsr_builtins ();
> +
>     tree ftype_jcvt
>       = build_function_type_list (intSI_type_node, double_type_node, NULL);
>     aarch64_builtin_decls[AARCH64_JSCVT]
> @@ -2599,6 +2671,123 @@ aarch64_expand_rng_builtin (tree exp, rtx target, int fcode, int ignore)
>     return target;
>   }
>   
> +/* Expand the read/write system register builtin EXPs.  */
> +rtx
> +aarch64_expand_rwsr_builtin (tree exp, rtx target, int fcode)
> +{
> +  tree arg0, arg1;
> +  rtx const_str, input_val, subreg;
> +  enum machine_mode mode;
> +  class expand_operand ops[2];
> +
> +  arg0 = CALL_EXPR_ARG (exp, 0);
> +
> +  bool write_op = (fcode == AARCH64_WSR
> +		   || fcode == AARCH64_WSRP
> +		   || fcode == AARCH64_WSR64
> +		   || fcode == AARCH64_WSRF
> +		   || fcode == AARCH64_WSRF64);
> +  bool read_op = (fcode == AARCH64_RSR
> +		  || fcode == AARCH64_RSRP
> +		  || fcode == AARCH64_RSR64
> +		  || fcode == AARCH64_RSRF
> +		  || fcode == AARCH64_RSRF64);
> +
> +  /* Argument 0 (system register name) must be a string literal.  */
> +  gcc_assert (!SSA_VAR_P (arg0)
> +	      && TREE_CODE (TREE_TYPE (arg0)) == POINTER_TYPE
> +	      && TREE_CODE (TREE_OPERAND (arg0,0)) == STRING_CST);
> +
> +  const char *name_input = TREE_STRING_POINTER (TREE_OPERAND (arg0, 0));
> +  size_t len = strlen (name_input);
> +
> +  if (len == 0)
> +    {
> +      error_at (EXPR_LOCATION (exp), "invalid system register name provided");
> +      return const0_rtx;
> +    }
> +
> +  char *sysreg_name = (char *) alloca (len + 1);
> +  strcpy (sysreg_name, name_input);
> +
> +  for (unsigned pos = 0; pos <= len; pos++)
> +    sysreg_name[pos] = TOLOWER (sysreg_name[pos]);
> +
> +  const char* name_output = aarch64_retrieve_sysreg (sysreg_name, write_op);
> +  if (name_output == NULL)
> +    {
> +      error_at (EXPR_LOCATION (exp), "invalid system register name provided");
> +      return const0_rtx;
> +    }
> +
> +  /* Assign the string corresponding to the system register name to an RTX.  */
> +  const_str = rtx_alloc (CONST_STRING);
> +  PUT_CODE (const_str, CONST_STRING);
> +  XSTR (const_str, 0) = xstrdup (name_output);
> +
> +  /* Set up expander operands and call instruction expansion.  */
> +  if (read_op)
> +    {
> +
> +      /* Emit the initial read_sysregdi rtx.  */
> +      create_output_operand (&ops[0], target, DImode);
> +      create_fixed_operand (&ops[1], const_str);
> +      expand_insn (CODE_FOR_aarch64_read_sysregdi, 2, ops);
> +      target = ops[0].value;
> +
> +      /* Do any necessary post-processing on the result.  */
> +      switch (fcode)
> +	{
> +	case AARCH64_RSR:
> +	  return gen_lowpart_SUBREG (SImode, target);
> +	case AARCH64_RSRP:
> +	case AARCH64_RSR64:
> +	  return target;
> +	case AARCH64_RSRF:
> +	  subreg = gen_reg_rtx (SImode);
> +	  subreg = gen_lowpart_SUBREG (SImode, target);
> +	  return gen_lowpart_SUBREG (SFmode, subreg);
> +	case AARCH64_RSRF64:
> +	  return gen_lowpart_SUBREG (DFmode, target);
> +	}
> +    }
> +  if (write_op)
> +    {
> +
> +      arg1 = CALL_EXPR_ARG (exp, 1);
> +      mode = TYPE_MODE (TREE_TYPE (arg1));
> +      input_val = copy_to_mode_reg (mode, expand_normal (arg1));
> +
> +      switch (fcode)
> +	{
> +	case AARCH64_WSR:
> +	  subreg = gen_lowpart_SUBREG (DImode, input_val);
> +	  break;
> +	case AARCH64_WSRP:
> +	case AARCH64_WSR64:
> +	  subreg = input_val;
> +	  break;
> +	case AARCH64_WSRF:
> +	  subreg = gen_lowpart_SUBREG (SImode, input_val);
> +	  subreg = gen_lowpart_SUBREG (DImode, subreg);
> +	  break;
> +	case AARCH64_WSRF64:
> +	  subreg = gen_lowpart_SUBREG (DImode, input_val);
> +	  break;
> +	}
> +
> +      create_fixed_operand (&ops[0], const_str);
> +      create_input_operand (&ops[1], subreg, DImode);
> +      expand_insn (CODE_FOR_aarch64_write_sysregdi, 2, ops);
> +
> +      return target;
> +    }
> +
> +  error_at (EXPR_LOCATION (exp),
> +	    "Malformed call to read/write system register builtin");
> +  return target;
> +}
> +
>   /* Expand an expression EXP that calls a MEMTAG built-in FCODE
>      with result going to TARGET.  */
>   static rtx
> @@ -2832,6 +3021,17 @@ aarch64_general_expand_builtin (unsigned int fcode, tree exp, rtx target,
>       case AARCH64_BUILTIN_RNG_RNDR:
>       case AARCH64_BUILTIN_RNG_RNDRRS:
>         return aarch64_expand_rng_builtin (exp, target, fcode, ignore);
> +    case AARCH64_RSR:
> +    case AARCH64_RSRP:
> +    case AARCH64_RSR64:
> +    case AARCH64_RSRF:
> +    case AARCH64_RSRF64:
> +    case AARCH64_WSR:
> +    case AARCH64_WSRP:
> +    case AARCH64_WSR64:
> +    case AARCH64_WSRF:
> +    case AARCH64_WSRF64:
> +      return aarch64_expand_rwsr_builtin (exp, target, fcode);
>       }
>   
>     if (fcode >= AARCH64_SIMD_BUILTIN_BASE && fcode <= AARCH64_SIMD_BUILTIN_MAX)
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 32c7adc8928..80da30bc30c 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -281,6 +281,8 @@
>       UNSPEC_UPDATE_FFRT
>       UNSPEC_RDFFR
>       UNSPEC_WRFFR
> +    UNSPEC_SYSREG_RDI
> +    UNSPEC_SYSREG_WDI
>       ;; Represents an SVE-style lane index, in which the indexing applies
>       ;; within the containing 128-bit block.
>       UNSPEC_SVE_LANE_SELECT
> @@ -476,6 +478,21 @@
>   ;; Jumps and other miscellaneous insns
>   ;; -------------------------------------------------------------------
>   
> +(define_insn "aarch64_read_sysregdi"
> + [(set (match_operand:DI 0 "register_operand" "=r")
> +       (unspec_volatile:DI [(match_operand 1 "aarch64_sysreg_string" "")]
> +		  UNSPEC_SYSREG_RDI))]
> + ""
> + "mrs\t%x0, %1"
> + )
> +
> +(define_insn "aarch64_write_sysregdi"
> + [(unspec_volatile:DI [(match_operand 0 "aarch64_sysreg_string" "")
> +	      (match_operand:DI 1 "register_operand" "r")] UNSPEC_SYSREG_WDI)]
> + ""
> + "msr\t%0, %x1"
> + )
> +
>   (define_insn "indirect_jump"
>     [(set (pc) (match_operand:DI 0 "register_operand" "r"))]
>     ""
> diff --git a/gcc/config/aarch64/arm_acle.h b/gcc/config/aarch64/arm_acle.h
> index 7599a32301d..71ada878299 100644
> --- a/gcc/config/aarch64/arm_acle.h
> +++ b/gcc/config/aarch64/arm_acle.h
> @@ -314,6 +314,36 @@ __rndrrs (uint64_t *__res)
>   
>   #pragma GCC pop_options
>   
> +#define __arm_rsr(__regname) \
> +  __builtin_aarch64_rsr (__regname)
> +
> +#define __arm_rsrp(__regname) \
> +  __builtin_aarch64_rsrp (__regname)
> +
> +#define __arm_rsr64(__regname) \
> +  __builtin_aarch64_rsr64 (__regname)
> +
> +#define __arm_rsrf(__regname) \
> +  __builtin_aarch64_rsrf (__regname)
> +
> +#define __arm_rsrf64(__regname) \
> +  __builtin_aarch64_rsrf64 (__regname)
> +
> +#define __arm_wsr(__regname, __value) \
> +  __builtin_aarch64_wsr (__regname, __value)
> +
> +#define __arm_wsrp(__regname, __value) \
> +  __builtin_aarch64_wsrp (__regname, __value)
> +
> +#define __arm_wsr64(__regname, __value) \
> +  __builtin_aarch64_wsr64 (__regname, __value)
> +
> +#define __arm_wsrf(__regname, __value) \
> +  __builtin_aarch64_wsrf (__regname, __value)
> +
> +#define __arm_wsrf64(__regname, __value) \
> +  __builtin_aarch64_wsrf64 (__regname, __value)
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/gcc/testsuite/gcc.target/aarch64/acle/rwsr-1.c b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-1.c
> new file mode 100644
> index 00000000000..bc9db098f16
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-1.c
> @@ -0,0 +1,20 @@
> +/* Test the __arm_[r,w]sr ACLE intrinsics family.  */
> +/* Ensure that illegal behavior is rejected by the compiler.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -march=armv8.4-a" } */
> +
> +#include <arm_acle.h>
> +
> +/* Ensure that read/write-only register attributes are respected by the compiler.  */
> +void
> +test_rwsr_read_write_only ()
> +{
> +  /* Attempt to write to read-only registers.  */
> +  long long a = __arm_rsr64 ("aidr_el1");  /* Read ok.  */
> +  __arm_wsr64 ("aidr_el1", a); /* { dg-error {invalid system register name provided} } */
> +
> +  /* Attempt to read from write-only registers.  */
> +  __arm_wsr64("icc_asgi1r_el1", a);            /* Write ok.  */
> +  long long b = __arm_rsr64("icc_asgi1r_el1"); /* { dg-error {invalid system register name provided} } */
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/acle/rwsr.c b/gcc/testsuite/gcc.target/aarch64/acle/rwsr.c
> new file mode 100644
> index 00000000000..3af4b960306
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/acle/rwsr.c
> @@ -0,0 +1,144 @@
> +/* Test the __arm_[r,w]sr ACLE intrinsics family.  */
> +/* Check that function variants for different data types handle types correctly.  */
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -march=armv8.4-a" } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +#include <arm_acle.h>
> +
> +/*
> +** get_rsr:
> +** ...
> +** 	mrs	x([0-9]+), s2_1_c0_c7_4
> +** 	add	w\1, w\1, 1
> +** ...
> +*/
> +int
> +get_rsr ()
> +{
> +  int a =  __arm_rsr("trcseqstr");
> +  return a+1;
> +}
> +
> +/*
> +** get_rsrf:
> +** 	mrs	x([0-9]+), s2_1_c0_c7_4
> +** 	fmov	s[0-9]+, w\1
> +** ...
> +*/
> +float
> +get_rsrf ()
> +{
> +  return __arm_rsrf("trcseqstr");
> +}
> +
> +/*
> +** get_rsrp:
> +** 	mrs	x0, s2_1_c0_c7_4
> +** 	ret
> +*/
> +void *
> +get_rsrp ()
> +{
> +  return __arm_rsrp("trcseqstr");
> +}
> +
> +/*
> +** get_rsr64:
> +** 	mrs	x0, s2_1_c0_c7_4
> +** 	ret
> +*/
> +long long
> +get_rsr64 ()
> +{
> +  return __arm_rsr64("trcseqstr");
> +}
> +
> +/*
> +** get_rsrf64:
> +** 	mrs	x([0-9]+), s2_1_c0_c7_4
> +** 	fmov	d[0-9]+, x\1
> +** ...
> +*/
> +double
> +get_rsrf64 ()
> +{
> +  return __arm_rsrf64("trcseqstr");
> +}
> +
> +/*
> +** set_wsr32:
> +** ...
> +** 	add	w([0-9]+), w\1, 1
> +** 	msr	s2_1_c0_c7_4, x\1
> +** ...
> +*/
> +void
> +set_wsr32 (int a)
> +{
> +  __arm_wsr("trcseqstr", a+1);
> +}
> +
> +/*
> +** set_wsrp:
> +** ...
> +** 	msr	s2_1_c0_c7_4, x[0-9]+
> +** ...
> +*/
> +void
> +set_wsrp (void *a)
> +{
> +  __arm_wsrp("trcseqstr", a);
> +}
> +
> +/*
> +** set_wsr64:
> +** ...
> +** 	msr	s2_1_c0_c7_4, x[0-9]+
> +** ...
> +*/
> +void
> +set_wsr64 (long long a)
> +{
> +  __arm_wsr64("trcseqstr", a);
> +}
> +
> +/*
> +** set_wsrf32:
> +** ...
> +** 	fmov	w([0-9]+), s[0-9]+
> +** 	msr	s2_1_c0_c7_4, x\1
> +** ...
> +*/
> +void
> +set_wsrf32 (float a)
> +{
> +  __arm_wsrf("trcseqstr", a);
> +}
> +
> +/*
> +** set_wsrf64:
> +** ...
> +** 	fmov	x([0-9]+), d[0-9]+
> +** 	msr	s2_1_c0_c7_4, x\1
> +** ...
> +*/
> +void
> +set_wsrf64(double a)
> +{
> +  __arm_wsrf64("trcseqstr", a);
> +}
> +
> +/*
> +** set_custom:
> +** ...
> +** 	mrs	x0, s1_2_c3_c4_5
> +** ...
> +** 	msr	s1_2_c3_c4_5, x0
> +** ...
> +*/
> +void set_custom()
> +{
> +  __uint64_t b = __arm_rsr64("S1_2_C3_C4_5");
> +  __arm_wsr64("S1_2_C3_C4_5", b);
> +}

OK.

Reviewed-by: rearnsha@arm.com