Richard Sandiford <richard.sandif...@arm.com> writes:
> Andrea Corallo <andrea.cora...@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
Thanks for the review, I'll update the patch.
Andrea
