Re: [PATCH v3 2/3] aarch64: Add support for moving fpm system register

[Claudio Bantaloukas <Claudio.Bantaloukas@arm.com>]

On 29/07/2024 13:13, Richard Sandiford wrote:
> Claudio Bantaloukas <claudio.bantaloukas@arm.com> writes:
>> Unlike most system registers, fpmr can be heavily written to in code that
>> exercises the fp8 functionality. That is because every fp8 instrinsic call
>> can potentially change the value of fpmr.
>> Rather than just use a an unspec, we treat the fpmr system register like
> 
> Typo: s/a an/an/
Thanks for the catch, will repost along with the requested changes below

Cheers,
Claudio

> 
>> all other registers and use a move operation to read and write to it.
>>
>> We introduce a new class of moveable system registers that, currently,
>> only accepts fpmr and a new constraint, Umv, that allows us to
>> selectively use mrs and msr instructions when expanding rtl for them.
>> Given that there is code that depends on "real" registers coming before
>> "fake" ones, we introduce a new constant FPM_REGNUM that uses an
>> existing value and renumber registers below that.
>> This requires us to update the bitmaps that describe which registers
>> belong to each register class.
>>
>> gcc/ChangeLog:
>>
>> 	* config/aarch64/aarch64.cc (aarch64_hard_regno_nregs): Add
>> 	support for MOVEABLE_SYSREGS class.
>> 	(aarch64_hard_regno_mode_ok): Allow reads and writes to fpmr.
>> 	(aarch64_regno_regclass): Support MOVEABLE_SYSREGS class.
>> 	(aarch64_class_max_nregs): Likewise.
>> 	* config/aarch64/aarch64.h (FIXED_REGISTERS): add fpmr.
>> 	(CALL_REALLY_USED_REGISTERS): Likewise.
>> 	(REGISTER_NAMES): Likewise.
>> 	(enum reg_class): Add MOVEABLE_SYSREGS class.
>> 	(REG_CLASS_NAMES): Likewise.
>> 	(REG_CLASS_CONTENTS): Update class bitmaps to deal with fpmr,
>> 	the new MOVEABLE_REGS class and renumbering of registers.
>> 	* config/aarch64/aarch64.md: (FPM_REGNUM): added new register
>> 	number, reusing old value.
>> 	(FFR_REGNUM): Renumber.
>> 	(FFRT_REGNUM): Likewise.
>> 	(LOWERING_REGNUM): Likewise.
>> 	(TPIDR2_BLOCK_REGNUM): Likewise.
>> 	(SME_STATE_REGNUM): Likewise.
>> 	(TPIDR2_SETUP_REGNUM): Likewise.
>> 	(ZA_FREE_REGNUM): Likewise.
>> 	(ZA_SAVED_REGNUM): Likewise.
>> 	(ZA_REGNUM): Likewise.
>> 	(ZT0_REGNUM): Likewise.
>> 	(*mov<mode>_aarch64): Add support for moveable sysregs.
>> 	(*movsi_aarch64): Likewise.
>> 	(*movdi_aarch64): Likewise.
>> 	* config/aarch64/constraints.md (MOVEABLE_SYSREGS): New constraint.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* gcc.target/aarch64/acle/fp8.c: New tests.
>> [...]
>> @@ -1405,6 +1409,8 @@ (define_insn "*mov<mode>_aarch64"
>>        [w, r Z  ; neon_from_gp<q>, nosimd     ] fmov\t%s0, %w1
>>        [w, w    ; neon_dup       , simd       ] dup\t%<Vetype>0, %1.<v>[0]
>>        [w, w    ; neon_dup       , nosimd     ] fmov\t%s0, %s1
>> +     [Umv, r  ; mrs            , *          ] msr\t%0, %x1
>> +     [r, Umv  ; mrs            , *          ] mrs\t%x0, %1
>>     }
>>   )
>>   
>> @@ -1467,6 +1473,8 @@ (define_insn_and_split "*movsi_aarch64"
>>        [r  , w  ; f_mrc    , fp  , 4] fmov\t%w0, %s1
>>        [w  , w  ; fmov     , fp  , 4] fmov\t%s0, %s1
>>        [w  , Ds ; neon_move, simd, 4] << aarch64_output_scalar_simd_mov_immediate (operands[1], SImode);
>> +     [Umv, r  ; mrs      , *   , 8] msr\t%0, %x1
>> +     [r, Umv  ; mrs      , *   , 8] mrs\t%x0, %1
> 
> The lengths should be 4 rather than 8.
> 
>>     }
>>     "CONST_INT_P (operands[1]) && !aarch64_move_imm (INTVAL (operands[1]), SImode)
>>       && REG_P (operands[0]) && GP_REGNUM_P (REGNO (operands[0]))"
>> @@ -1505,6 +1513,8 @@ (define_insn_and_split "*movdi_aarch64"
>>        [w, w  ; fmov     , fp  , 4] fmov\t%d0, %d1
>>        [w, Dd ; neon_move, simd, 4] << aarch64_output_scalar_simd_mov_immediate (operands[1], DImode);
>>        [w, Dx ; neon_move, simd, 8] #
>> +     [Umv, r; mrs      , *   , 8] msr\t%0, %1
>> +     [r, Umv; mrs      , *   , 8] mrs\t%0, %1
> 
> Similarly here.
> 
>>     }
>>     "CONST_INT_P (operands[1])
>>      && REG_P (operands[0])
>> [...]
>> diff --git a/gcc/testsuite/gcc.target/aarch64/acle/fp8.c b/gcc/testsuite/gcc.target/aarch64/acle/fp8.c
>> index 459442be155..1a5c3d7e8fd 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/acle/fp8.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/acle/fp8.c
>> @@ -1,6 +1,7 @@
>>   /* Test the fp8 ACLE intrinsics family.  */
>>   /* { dg-do compile } */
>>   /* { dg-options "-O1 -march=armv8-a" } */
>> +/* { dg-final { check-function-bodies "**" "" "" } } */
>>   
>>   #include <arm_acle.h>
>>   
>> @@ -17,4 +18,107 @@
>>   #error "__ARM_FEATURE_FP8 feature macro defined."
>>   #endif
>>   
>> +/*
>> +**test_write_fpmr_sysreg_asm_64:
>> +**	msr	fpmr, x0
>> +**	ret
>> +*/
>> +void
>> +test_write_fpmr_sysreg_asm_64 (uint64_t val)
>> +{
>> +  register uint64_t fpmr asm ("fpmr") = val;
>> +  asm volatile ("" ::"Umv"(fpmr));
>> +}
>> +
>> +/*
>> +**test_write_fpmr_sysreg_asm_32:
>> +**	uxtw	x0, w0
>> +**	msr	fpmr, x0
>> +**	ret
>> +*/
>> +void
>> +test_write_fpmr_sysreg_asm_32 (uint32_t val)
>> +{
>> +  register uint64_t fpmr asm ("fpmr") = val;
> 
> By using uint64_t rather than uint32_t, these tests are testing movdi
> rather than the smaller move patterns.  I think it should be uint32_t
> instead.  We should then have just an MSR, without an extension.
> 
> Similarly for the 16-bit and 8-bit cases.
> 
> LGTM with those changes, but please give others a day or so to comment.
> 
> Thanks,
> Richard
> 
> 
>> +  asm volatile ("" ::"Umv"(fpmr));
>> +}
>> +
>> +/*
>> +**test_write_fpmr_sysreg_asm_16:
>> +**	and	x0, x0, 65535
>> +**	msr	fpmr, x0
>> +**	ret
>> +*/
>> +void
>> +test_write_fpmr_sysreg_asm_16 (uint16_t val)
>> +{
>> +  register uint64_t fpmr asm ("fpmr") = val;
>> +  asm volatile ("" ::"Umv"(fpmr));
>> +}
>> +
>> +/*
>> +**test_write_fpmr_sysreg_asm_8:
>> +**	and	x0, x0, 255
>> +**	msr	fpmr, x0
>> +**	ret
>> +*/
>> +void
>> +test_write_fpmr_sysreg_asm_8 (uint8_t val)
>> +{
>> +  register uint64_t fpmr asm ("fpmr") = val;
>> +  asm volatile ("" ::"Umv"(fpmr));
>> +}
>> +
>> +/*
>> +**test_read_fpmr_sysreg_asm_64:
>> +**	mrs	x0, fpmr
>> +**	ret
>> +*/
>> +uint64_t
>> +test_read_fpmr_sysreg_asm_64 ()
>> +{
>> +  register uint64_t fpmr asm ("fpmr");
>> +  asm volatile ("" : "=Umv"(fpmr) :);
>> +  return fpmr;
>> +}
>> +
>> +/*
>> +**test_read_fpmr_sysreg_asm_32:
>> +**	mrs	x0, fpmr
>> +**	ret
>> +*/
>> +uint32_t
>> +test_read_fpmr_sysreg_asm_32 ()
>> +{
>> +  register uint32_t fpmr asm ("fpmr");
>> +  asm volatile ("" : "=Umv"(fpmr) :);
>> +  return fpmr;
>> +}
>> +
>> +/*
>> +**test_read_fpmr_sysreg_asm_16:
>> +**	mrs	x0, fpmr
>> +**	ret
>> +*/
>> +uint16_t
>> +test_read_fpmr_sysreg_asm_16 ()
>> +{
>> +  register uint16_t fpmr asm ("fpmr");
>> +  asm volatile ("" : "=Umv"(fpmr) :);
>> +  return fpmr;
>> +}
>> +
>> +/*
>> +**test_read_fpmr_sysreg_asm_8:
>> +**	mrs	x0, fpmr
>> +**	ret
>> +*/
>> +uint8_t
>> +test_read_fpmr_sysreg_asm_8 ()
>> +{
>> +  register uint8_t fpmr asm ("fpmr");
>> +  asm volatile ("" : "=Umv"(fpmr) :);
>> +  return fpmr;
>> +}
>> +
>>   #pragma GCC pop_options