diff mbox series

Reset HWCAP2_AFP bits in FPCR for default fenv.

Message ID AS8PR08MB7079624B79439292B360CCAAEABB9@AS8PR08MB7079.eurprd08.prod.outlook.com
State Superseded
Headers
Series Reset HWCAP2_AFP bits in FPCR for default fenv. |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Tejas Belagod June 29, 2022, 9:34 a.m. UTC 
  Hi,

The AFP feature (Alternate floating-point behavior) was added in Armv8.7-A and
introduced new FPCR bits.

Currently, HWCAP2_AFP bits (bit 0, 1, 2) in FPCR are preserved when fenv is
set to default environment.  This is a deviation from standard behavior.
Clear these bits when setting the fenv to default.

There is no libc API to modify the new FPCR bits.  Restoring those bits matters
if the user changed them directly.

OK for master?

Thanks,
Tejas.

Comments

Florian Weimer June 29, 2022, 9:54 a.m. UTC | #1 
* Tejas Belagod via Libc-alpha:

> The AFP feature (Alternate floating-point behavior) was added in
> Armv8.7-A and introduced new FPCR bits.
>
> Currently, HWCAP2_AFP bits (bit 0, 1, 2) in FPCR are preserved when
> fenv is set to default environment.  This is a deviation from standard
> behavior.  Clear these bits when setting the fenv to default.
>
> There is no libc API to modify the new FPCR bits.  Restoring those
> bits matters if the user changed them directly.
>
> OK for master?

What do you propose as commit message?

I think we have that definition of _FPU_RESERVED because the we cannot
know the semantics of those bits (obviosuly), and changing them
underneath the user's code could be wrong.

What do these bits do, actually?  If this adds additional FPU state, how
does it interact with compiler behavior and optimizations?  Can it even
be safely changed after process startup?

Thanks,
Florian
Szabolcs Nagy June 29, 2022, 11:24 a.m. UTC | #2 
The 06/29/2022 11:54, Florian Weimer wrote:
> * Tejas Belagod via Libc-alpha:
> 
> > The AFP feature (Alternate floating-point behavior) was added in
> > Armv8.7-A and introduced new FPCR bits.
> >
> > Currently, HWCAP2_AFP bits (bit 0, 1, 2) in FPCR are preserved when
> > fenv is set to default environment.  This is a deviation from standard
> > behavior.  Clear these bits when setting the fenv to default.
> >
> > There is no libc API to modify the new FPCR bits.  Restoring those
> > bits matters if the user changed them directly.
> >
> > OK for master?
> 
> What do you propose as commit message?
> 
> I think we have that definition of _FPU_RESERVED because the we cannot
> know the semantics of those bits (obviosuly), and changing them
> underneath the user's code could be wrong.
> 
> What do these bits do, actually?  If this adds additional FPU state, how
> does it interact with compiler behavior and optimizations?  Can it even
> be safely changed after process startup?

i think you will have to check the details in the arm arm
https://developer.arm.com/documentation/ddi0487/latest

in short, the new bits

FPCR.NEP: changes output of scalar fp instructions in top vector lanes.
FPCR.AH: changes flush to zero exception, underflow and nan propagation.
FPCR.FIZ: subnormal inputs are flushed to zero (instead of outputs).

(note that the alternate fp behaviour is carefully designed so
it's easy to emulate fp instructions of other architectures, i.e.
the bits are likely not used to change math library behaviour but
to execute a foreign binary on aarch64.)

technically these should be 0 in c code (then save/restore is noop).
it seems the existing reserved bits are not consistent:

- DN is reserved (when set, default nan is returned instead of
  propagating nan payload)

- FZ is not reserved (subnormal flush to zero)

These are non-conforming settings so fenv apis could ignore both
or save/restore both.

i don't have a strong opinion, but i thought it made sense to
restore all these special bits with fesetenv(FE_DFL_ENV).
(e.g. in an async signal handler one could do

  fegetenv(&e);
  fesetenv(FE_DFL_ENV);
  // fp operations
  fesetenv(&e);

to get sane fenv. this is ub in iso c, but not unreasonable.)
Joseph Myers June 29, 2022, 4:53 p.m. UTC | #3 
On Wed, 29 Jun 2022, Szabolcs Nagy via Libc-alpha wrote:

> i don't have a strong opinion, but i thought it made sense to
> restore all these special bits with fesetenv(FE_DFL_ENV).

Yes, I think FE_DFL_ENV (and FE_DFL_MODE) should cover all such 
architecture-specific control settings, even when we also don't support 
the use of most floating-point APIs with non-default values of those 
settings.
Tejas Belagod June 30, 2022, 10:19 a.m. UTC | #4 
> -----Original Message-----
> From: Joseph Myers <joseph@codesourcery.com>
> Sent: Wednesday, June 29, 2022 10:24 PM
> To: Szabolcs Nagy <Szabolcs.Nagy@arm.com>
> Cc: Florian Weimer <fweimer@redhat.com>; Tejas Belagod
> <Tejas.Belagod@arm.com>; Tejas Belagod via Libc-alpha <libc-
> alpha@sourceware.org>
> Subject: Re: Reset HWCAP2_AFP bits in FPCR for default fenv.
> 
> On Wed, 29 Jun 2022, Szabolcs Nagy via Libc-alpha wrote:
> 
> > i don't have a strong opinion, but i thought it made sense to restore
> > all these special bits with fesetenv(FE_DFL_ENV).
> 
> Yes, I think FE_DFL_ENV (and FE_DFL_MODE) should cover all such
> architecture-specific control settings, even when we also don't support the
> use of most floating-point APIs with non-default values of those settings.
> 

Thanks all for your comments. 

Florian, given the explanations from Szabolcs and Joseph, do you have any objections to this change?

Thanks,
Tejas.

> --
> Joseph S. Myers
> joseph@codesourcery.com
Florian Weimer June 30, 2022, 10:42 a.m. UTC | #5 
* Tejas Belagod:

>> -----Original Message-----
>> From: Joseph Myers <joseph@codesourcery.com>
>> Sent: Wednesday, June 29, 2022 10:24 PM
>> To: Szabolcs Nagy <Szabolcs.Nagy@arm.com>
>> Cc: Florian Weimer <fweimer@redhat.com>; Tejas Belagod
>> <Tejas.Belagod@arm.com>; Tejas Belagod via Libc-alpha <libc-
>> alpha@sourceware.org>
>> Subject: Re: Reset HWCAP2_AFP bits in FPCR for default fenv.
>> 
>> On Wed, 29 Jun 2022, Szabolcs Nagy via Libc-alpha wrote:
>> 
>> > i don't have a strong opinion, but i thought it made sense to restore
>> > all these special bits with fesetenv(FE_DFL_ENV).
>> 
>> Yes, I think FE_DFL_ENV (and FE_DFL_MODE) should cover all such
>> architecture-specific control settings, even when we also don't support the
>> use of most floating-point APIs with non-default values of those settings.
>> 
>
> Thanks all for your comments. 
>
> Florian, given the explanations from Szabolcs and Joseph, do you have
> any objections to this change?

No objection to the change itself, but you didn't post the proposed
commit message.  Please mention “aarch64” in the subject.

Thanks,
Florian
Tejas Belagod June 30, 2022, 10:56 a.m. UTC | #6 
Thanks. 

Sorry, I intended the original mail body without the 'OK for master?' as the commit message. I shall re-post the patch with the correct formatting.

Thanks,
Tejas.

> -----Original Message-----
> From: Florian Weimer <fweimer@redhat.com>
> Sent: Thursday, June 30, 2022 4:13 PM
> To: Tejas Belagod <Tejas.Belagod@arm.com>
> Cc: Joseph Myers <joseph@codesourcery.com>; Szabolcs Nagy
> <Szabolcs.Nagy@arm.com>; Tejas Belagod via Libc-alpha <libc-
> alpha@sourceware.org>
> Subject: Re: Reset HWCAP2_AFP bits in FPCR for default fenv.
> 
> * Tejas Belagod:
> 
> >> -----Original Message-----
> >> From: Joseph Myers <joseph@codesourcery.com>
> >> Sent: Wednesday, June 29, 2022 10:24 PM
> >> To: Szabolcs Nagy <Szabolcs.Nagy@arm.com>
> >> Cc: Florian Weimer <fweimer@redhat.com>; Tejas Belagod
> >> <Tejas.Belagod@arm.com>; Tejas Belagod via Libc-alpha <libc-
> >> alpha@sourceware.org>
> >> Subject: Re: Reset HWCAP2_AFP bits in FPCR for default fenv.
> >>
> >> On Wed, 29 Jun 2022, Szabolcs Nagy via Libc-alpha wrote:
> >>
> >> > i don't have a strong opinion, but i thought it made sense to
> >> > restore all these special bits with fesetenv(FE_DFL_ENV).
> >>
> >> Yes, I think FE_DFL_ENV (and FE_DFL_MODE) should cover all such
> >> architecture-specific control settings, even when we also don't
> >> support the use of most floating-point APIs with non-default values of
> those settings.
> >>
> >
> > Thanks all for your comments.
> >
> > Florian, given the explanations from Szabolcs and Joseph, do you have
> > any objections to this change?
> 
> No objection to the change itself, but you didn't post the proposed commit
> message.  Please mention “aarch64” in the subject.
> 
> Thanks,
> Florian
diff mbox series

Patch

diff --git a/sysdeps/aarch64/fpu/fpu_control.h b/sysdeps/aarch64/fpu/fpu_control.h
index 764ed5cdbb6a90a4d6ac9af1f8874fd71c379e62..429f4910e7a165a7ba8170b173c4fbb3960afa3f 100644
--- a/sysdeps/aarch64/fpu/fpu_control.h
+++ b/sysdeps/aarch64/fpu/fpu_control.h
@@ -46,7 +46,7 @@ 
    contents. These two masks indicate which bits in each of FPCR and
    FPSR should not be changed.  */
 
-#define _FPU_RESERVED		0xfe0fe0ff
+#define _FPU_RESERVED		0xfe0fe0f8
 #define _FPU_FPSR_RESERVED	0x0fffffe0
 
 #define _FPU_DEFAULT		0x00000000