On 11/03/2020 13:53, Richard Earnshaw (lists) wrote:
> On 11/03/2020 13:50, Szabolcs Nagy wrote:
>> On 10/03/2020 09:55, Andrea Corallo wrote:
>>> Hi all,
>>>
>>> second and last patch of the two reworking FPCR and FPSR builtins.
>>>
>>> This rework __builtin_aarch64_set_fpcr (unsigned) and
>>> __builtin_aarch64_set_fpsr (unsigned) to emit a read-modify-sequences
>>> as:
>>>
>>>           mrs     x1, fpsr
>>>           bfi     x1, x0, 0, 32
>>>           msr     fpsr, x1
>>>
>>> This in order to preserve the original high 32 bits of the system
>>> register.  Both FPSR and FPCR became 64bit regs with armv8.1.
>>
>> so the architectural fpsr and fpcr registers got
>> extended to 64bit from 32bit and currently the top
>> 32 bits are reserved 0.
>>
>> the floating-point environment has to fit in fenv_t
>> which is currently two unsigned ints on aarch64.
>>
>> so glibc cannot support 64bit registers without breaking
>> abi which means in c programs the fpsr, fpcr top bits
>> must be held 0 at all times when extern calls are made
>> that involves dynamic linking or calls into the libc
>> (otherwise fegetenv and fesetenv are not able to
>> reliably restore the default fp environment).
> 
> I disagree.  The top bits must be preserved, even if they are not zero. This 
> does potentially limit what those top bits might be used for, since
> they are beyond the ability to represent state in C-like programming 
> environments, but it doesn't mean they should be reset by a call to change
> the register.


ok preserving works for fesetenv.
(which internally uses the builtin)

it means the top bits are not part of fenv.
(we will have to change asm in glibc to deal with this)

since the top and bottom bits cannot be set
independently there are implications about
atomic updates, but they are not relevant
on linux where signal handlers save/restore
the fenv (not required by the standard so
theoretically treating the top and bottom bits
separately may cause issues on some systems).

> 
> 
> R.
> 
>>
>> i think trying to preserve the top bits is wrong
>> (it may be acceptable depending on what those bits
>> do, but it seems to me better to only allow 0 top bits).
>>
>> if the user changes the top bits the behaviour is
>> undefined if an extern c call is made with such setting.
>>
>> i think we have to change the builtins to ensure the
>> top bits of the used x reg is 0. we may or may not need
>> a new builtin to allow setting 64 bits of fpsr and fpcr:
>> currently non-zero top bits are unsupportable (but once
>> there is new meaning for the top 32bits then it may make
>> sense to break abi).
>>
>>
>>>
>>> Bootstrapped on aarch64-linux-gnu, does not introduce regressions.
>>>
>>> Regards
>>>
>>>    Andrea
>>>
>>> gcc/ChangeLog:
>>>
>>> 2020-??-??  Andrea Corallo  <andrea.cora...@arm.com>
>>>
>>>     * config/aarch64/aarch64.md (insv_reg<mode>): Pattern renamed.
>>>     (set_fpcr, set_fpsr): Pattern modified for read-modify-write
>>>     sequence respecting high 32bit register content.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2020-??-??  Andrea Corallo  <andrea.cora...@arm.com>
>>>
>>>     * gcc.target/aarch64/set_fpcr.c: New test.
>>>     * gcc.target/aarch64/get_fpcr.c: New test.
>>>     * gcc.target/aarch64/set_fpsr.c: New test.
>>>     * gcc.target/aarch64/get_fpsr.c: New test.
>>>
>>
> 

Reply via email to