[PATCH 1/2] [aarch64] Rework fpcr fpsr getter/setter builtins

[Richard Sandiford]

Andrea Corallo <andrea.corallo@arm.com> writes:
> Hi all,
>
> I'd like to submit this patch introducing the following 64bit builtins
> variants as FPCR and FPSR registers getter/setter:
>
> unsigned long long __builtin_aarch64_get_fpcr64 ()
> void __builtin_aarch64_set_fpcr64 (unsigned long long)
> unsigned long long __builtin_aarch64_get_fpsr64 ()
> void __builtin_aarch64_set_fpsr64 (unsigned long long)

Sound like this part is uncontroversial.  At least, if anyone objects
to it, they should have said so earlier :-)

> @@ -1240,33 +1245,65 @@ aarch64_init_memtag_builtins (void)
>  #undef AARCH64_INIT_MEMTAG_BUILTINS_DECL
>  }
>  
> -/* Initialize all builtins in the AARCH64_BUILTIN_GENERAL group.  */
> +/* Initialize fpsr fpcr getter and setters.  */

“getters”

> @@ -1871,6 +1908,40 @@ aarch64_expand_builtin_memtag (int fcode, tree exp, rtx target)
>    return target;
>  }
>  
> +static rtx
> +aarch64_expand_fcr_fpsr_builtin (tree exp, machine_mode mode, bool getter,
> +				 bool fpsr)
> +{
> +  int icode;
> +  rtx pat;
> +  rtx target = NULL_RTX;
> +
> +  gcc_assert (mode == SImode || (mode == DImode));
> +
> +  if (getter)
> +    {
> +      if (mode == SImode)
> +	icode = fpsr ? CODE_FOR_get_fpsr : CODE_FOR_get_fpcr;
> +      else
> +	icode = fpsr ? CODE_FOR_get_fpsr64 : CODE_FOR_get_fpcr64;
> +      target = gen_reg_rtx (mode);
> +      pat = GEN_FCN (icode) (target);
> +    }
> +  else
> +    {
> +      target = NULL_RTX;
> +      if (mode == SImode)
> +	icode = fpsr ? CODE_FOR_set_fpsr : CODE_FOR_set_fpcr;
> +      else
> +	icode = fpsr ? CODE_FOR_set_fpsr64 : CODE_FOR_set_fpcr64;
> +      tree arg = CALL_EXPR_ARG (exp, 0);
> +      rtx op = force_reg (mode, expand_normal (arg));
> +      pat = GEN_FCN (icode) (op);
> +    }
> +  emit_insn (pat);
> +  return target;
> +}
> +
>  /* Expand an expression EXP that calls built-in function FCODE,
>     with result going to TARGET if that's convenient.  IGNORE is true
>     if the result of the builtin is ignored.  */
> @@ -1879,35 +1950,27 @@ aarch64_general_expand_builtin (unsigned int fcode, tree exp, rtx target,
>  				int ignore)
>  {
>    int icode;
> -  rtx pat, op0;
> +  rtx op0;
>    tree arg0;
>  
>    switch (fcode)
>      {
>      case AARCH64_BUILTIN_GET_FPCR:
> +      return aarch64_expand_fcr_fpsr_builtin (exp, SImode, true, false);
>      case AARCH64_BUILTIN_SET_FPCR:
> +      return aarch64_expand_fcr_fpsr_builtin (exp, SImode, false, false);
>      case AARCH64_BUILTIN_GET_FPSR:
> +      return aarch64_expand_fcr_fpsr_builtin (exp, SImode, true, true);
>      case AARCH64_BUILTIN_SET_FPSR:
> -      if ((fcode == AARCH64_BUILTIN_GET_FPCR)
> -	  || (fcode == AARCH64_BUILTIN_GET_FPSR))
> -	{
> -	  icode = (fcode == AARCH64_BUILTIN_GET_FPSR) ?
> -	    CODE_FOR_get_fpsr : CODE_FOR_get_fpcr;
> -	  target = gen_reg_rtx (SImode);
> -	  pat = GEN_FCN (icode) (target);
> -	}
> -      else
> -	{
> -	  target = NULL_RTX;
> -	  icode = (fcode == AARCH64_BUILTIN_SET_FPSR) ?
> -	    CODE_FOR_set_fpsr : CODE_FOR_set_fpcr;
> -	  arg0 = CALL_EXPR_ARG (exp, 0);
> -	  op0 = force_reg (SImode, expand_normal (arg0));
> -	  pat = GEN_FCN (icode) (op0);
> -	}
> -      emit_insn (pat);
> -      return target;
> -
> +      return aarch64_expand_fcr_fpsr_builtin (exp, SImode, false, true);
> +    case AARCH64_BUILTIN_GET_FPCR64:
> +      return aarch64_expand_fcr_fpsr_builtin (exp, DImode, true, false);
> +    case AARCH64_BUILTIN_SET_FPCR64:
> +      return aarch64_expand_fcr_fpsr_builtin (exp, DImode, false, false);
> +    case AARCH64_BUILTIN_GET_FPSR64:
> +      return aarch64_expand_fcr_fpsr_builtin (exp, DImode, true, true);
> +    case AARCH64_BUILTIN_SET_FPSR64:
> +      return aarch64_expand_fcr_fpsr_builtin (exp, DImode, false, true);
>      case AARCH64_PAUTH_BUILTIN_AUTIA1716:
>      case AARCH64_PAUTH_BUILTIN_PACIA1716:
>      case AARCH64_PAUTH_BUILTIN_AUTIB1716:
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 7ad4e918578b..b6836710c9c2 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -289,6 +289,10 @@
>      UNSPECV_SET_FPCR		; Represent assign of FPCR content.
>      UNSPECV_GET_FPSR		; Represent fetch of FPSR content.
>      UNSPECV_SET_FPSR		; Represent assign of FPSR content.
> +    UNSPECV_GET_FPCR64		; Represent fetch of 64 bits FPCR content.
> +    UNSPECV_SET_FPCR64		; Represent assign of 64 bits FPCR content.
> +    UNSPECV_GET_FPSR64		; Represent fetch of 64 bits FPSR content.
> +    UNSPECV_SET_FPSR64		; Represent assign of 64 bits FPSR content.
>      UNSPECV_BLOCKAGE		; Represent a blockage
>      UNSPECV_PROBE_STACK_RANGE	; Represent stack range probing.
>      UNSPECV_SPECULATION_BARRIER ; Represent speculation barrier.
> @@ -7198,6 +7202,35 @@
>    "mrs\\t%0, fpsr"
>    [(set_attr "type" "mrs")])
>  
> +;; Write 64 bits into Floating-point Control Register.
> +(define_insn "set_fpcr64"
> +  [(unspec_volatile [(match_operand:DI 0 "register_operand" "r")] UNSPECV_SET_FPCR64)]
> +  ""
> +  "msr\\tfpcr, %0"
> +  [(set_attr "type" "mrs")])
> +
> +;; Read Floating-point Control Register 64 bits.
> +(define_insn "get_fpcr64"
> +  [(set (match_operand:DI 0 "register_operand" "=r")
> +        (unspec_volatile:DI [(const_int 0)] UNSPECV_GET_FPCR64))]
> +  ""
> +  "mrs\\t%0, fpcr"
> +  [(set_attr "type" "mrs")])
> +
> +;; Write 64 bits into Floating-point Status Register.
> +(define_insn "set_fpsr64"
> +  [(unspec_volatile [(match_operand:DI 0 "register_operand" "r")] UNSPECV_SET_FPSR64)]
> +  ""
> +  "msr\\tfpsr, %0"
> +  [(set_attr "type" "mrs")])
> +
> +;; Read Floating-point Status Register 64 bits.
> +(define_insn "get_fpsr64"
> +  [(set (match_operand:DI 0 "register_operand" "=r")
> +        (unspec_volatile:DI [(const_int 0)] UNSPECV_GET_FPSR64))]
> +  ""
> +  "mrs\\t%0, fpsr"
> +  [(set_attr "type" "mrs")])

I think instead we should replace the existing and new patterns with
something like:

(define_insn "@aarch64_get_<GET_FPSCR:name><GPI:mode>"
  [(set (...:GPI ...)
        (unspec_volatile:GPI ... GET_FPSCR))]
  "mrs... <GET_FPSCR:name>"
  ...)

for the getter and similarly for the setter, giving 2 patterns in total.
GET_FPSCR would be a define_int_iterator iterating over UNSPECV_GET_FPSR
and UNSPECV_GET_FPCR.  We wouldn't need separate unspecs for the 64-bit
versions; that would be indicated by the mode instead.

Then aarch64_expand_fcr_fpsr_builtin splits naturally into two:
one for getters and one for setters.  The getter side would need:

  gen_arch64_get (unspec_id, mode, target)

where unspec_id is UNSPECV_GET_FPSR or UNSPECV_GET_FPCR.
Similarly for the setter side.

Thanks,
Richard