[PATCH v2] riscv: Add macros for FPUCW/fcsr in fpu_control.h

[PATCH v2] riscv: Add macros for FPUCW/fcsr in fpu_control.h

[Shiqi Zhang]

Add macros for rounding modes and accrued exception flags in order to
make controlling fcsr easier for users.

Reference: RISC-V Unprivileged Spec v20191213, Section 11.2: Figure 11.2, Table 11.1 & 11.2
---
Fixed careless mistakes in v1 and removed invalid rounding mode DYN.

Still, I think these macros should be documented somewhere but I'm
not sure where should I doc them. I'll appreciate it if you could give
some suggestions.

Regards,
Shiqi Zhang

 sysdeps/riscv/fpu_control.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sysdeps/riscv/fpu_control.h b/sysdeps/riscv/fpu_control.h
index c33798c6bb..1dcadd3ea1 100644
--- a/sysdeps/riscv/fpu_control.h
+++ b/sysdeps/riscv/fpu_control.h
@@ -36,6 +36,20 @@ extern fpu_control_t __fpu_control;
 # define _FPU_DEFAULT  0
 # define _FPU_IEEE     _FPU_DEFAULT
 
+/* FPU rounding modes */
+# define _FPU_RM_RNE (0 << 5)
+# define _FPU_RM_RTZ (1 << 5)
+# define _FPU_RM_RDN (2 << 5)
+# define _FPU_RM_RUP (3 << 5)
+# define _FPU_RM_RMM (4 << 5)
+
+/* FPU accrued exception flags */
+# define _FPU_EXCEPT_NV (1 << 4)
+# define _FPU_EXCEPT_DZ (1 << 3)
+# define _FPU_EXCEPT_OF (1 << 2)
+# define _FPU_EXCEPT_UF (1 << 1)
+# define _FPU_EXCEPT_NX (1 << 0)
+
 /* Type of the control word.  */
 typedef unsigned int fpu_control_t __attribute__ ((__mode__ (__SI__)));
 
-- 
2.39.1

Re: [PATCH v2] riscv: Add macros for FPUCW/fcsr in fpu_control.h

[Richard Henderson]

On 2/11/23 20:52, Shiqi Zhang wrote:
> Add macros for rounding modes and accrued exception flags in order to
> make controlling fcsr easier for users.
> 
> Reference: RISC-V Unprivileged Spec v20191213, Section 11.2: Figure 11.2, Table 11.1 & 11.2
> ---
> Fixed careless mistakes in v1 and removed invalid rounding mode DYN.
> 
> Still, I think these macros should be documented somewhere but I'm
> not sure where should I doc them. I'll appreciate it if you could give
> some suggestions.

Honestly, no one should be using <fpu_control.h> at all.  It was originally an x86 
specific thing.  The only reason to fill in this header at all is to match the original 
x86 API.  Therefore the symbol names should match ./sysdeps/x86/fpu_control.h.

When porting applications to work on RISC-V, it would be much better to update them to use 
<fenv.h>, which is what ISO C standardized.  I assume there would be more than a few 
ifdefs removed in the process.


r~

Re: [PATCH v2] riscv: Add macros for FPUCW/fcsr in fpu_control.h

[Shiqi Zhang]

> Honestly, no one should be using <fpu_control.h> at all.  It was
> originally an x86 specific thing.  The only reason to fill in this
> header at all is to match the original x86 API.  Therefore the symbol
> names should match ./sysdeps/x86/fpu_control.h.


Actually other architectures also defines these arch-specific macros in
./sysdeps/***/fpu_control.h. And the symbol names doesn't match x86,
too. For example I checked the macro _FPU_FPCR_MASK_OFE in
./sysdeps/aarch64/fpu/fpu_control.h, nothing in glibc referenced it so I
think it can only be for the users to control FPU in an arch-specific
manner.


>
> When porting applications to work on RISC-V, it would be much better
> to update them to use <fenv.h>, which is what ISO C standardized.  I
> assume there would be more than a few ifdefs removed in the process.
>

The rounding mode RMM (Round to Nearest, ties to Max Magnitude) of
RISC-V isn't part of the C standard. So I think it makes sense to define
these macros in arch-specific headers.

Re: [PATCH v2] riscv: Add macros for FPUCW/fcsr in fpu_control.h

[Andreas Schwab]

On Feb 13 2023, Shiqi Zhang wrote:

> The rounding mode RMM (Round to Nearest, ties to Max Magnitude) of
> RISC-V isn't part of the C standard.

Doesn't that match FE_TONEARESTFROMZERO?  In any case, an implementation
may add additional FE_ macros.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH v2] riscv: Add macros for FPUCW/fcsr in fpu_control.h

[Shiqi Zhang]

The fenv macro FE_TONEARESTFROMZERO doesn't exist yet and seems to
belong to a newer version of C standard draft. Maybe I should implement
that along with the macros in fpu_control.h?

I'm defining these macros here primarily because in most cases glibc is
the lowest layer of user programs. So I think it's the best place to
define these macros for directly interacting with arch-specific things
like FPU control, like other architectures have done.

At 2023/2/13 16:39, Andreas Schwab wrote:
> On Feb 13 2023, Shiqi Zhang wrote:
>
>> The rounding mode RMM (Round to Nearest, ties to Max Magnitude) of
>> RISC-V isn't part of the C standard.
> Doesn't that match FE_TONEARESTFROMZERO?  In any case, an implementation
> may add additional FE_ macros.
>

Re: [PATCH v2] riscv: Add macros for FPUCW/fcsr in fpu_control.h

[Joseph Myers]

On Mon, 13 Feb 2023, Shiqi Zhang wrote:

> The fenv macro FE_TONEARESTFROMZERO doesn't exist yet and seems to
> belong to a newer version of C standard draft. Maybe I should implement
> that along with the macros in fpu_control.h?

It's desirable in principle, but is pretty complicated to add (as alluded 
to in past discussions) - various code in the architecture-independent 
parts of glibc and tests knows about all the rounding modes and would need 
updating to handle this one (without being any less efficient in the 
common case of architectures not supporting this mode).  That includes, 
for example, strtod, printf and their tests, the gen-auto-libm-tests 
machinery and all the auto-libm-test-out-* files which would need 
regenerating, all the libm test machinery that deals with actually running 
tests over all rounding modes, various tests of <fenv.h> operation, and 
soft-fp (in glibc and then updating the copy in libgcc) so that binary128 
operations work properly when this is the hardware rounding mode.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v2] riscv: Add macros for FPUCW/fcsr in fpu_control.h

[Richard Henderson]

On 2/12/23 20:16, Shiqi Zhang wrote:
> 
>> Honestly, no one should be using <fpu_control.h> at all.  It was
>> originally an x86 specific thing.  The only reason to fill in this
>> header at all is to match the original x86 API.  Therefore the symbol
>> names should match ./sysdeps/x86/fpu_control.h.
> 
> 
> Actually other architectures also defines these arch-specific macros in
> ./sysdeps/***/fpu_control.h. And the symbol names doesn't match x86,
> too. For example I checked the macro _FPU_FPCR_MASK_OFE in
> ./sysdeps/aarch64/fpu/fpu_control.h, nothing in glibc referenced it so I
> think it can only be for the users to control FPU in an arch-specific
> manner.

Oh the other hand, note the number of targets that use

_FPU_RC_NEAREST, _FPU_RC_DOWN, _FPU_RC_UP, _FPU_RC_ZERO

x86, alpha, csky, loongarch, m68k, mips, ppc, sparc, sh (nearest, zero).

Indeed, arm and aarch64 seem to be the outliers in being completely different.


r~

Re: [PATCH v2] riscv: Add macros for FPUCW/fcsr in fpu_control.h

[Shiqi Zhang]

Oh, honestly I didn't notice the consistency in macros of rounding
modes. I only checked the exception masks and found them inconsistent
between architectures, so I decided to name them as defined in RISC-V
specs. Maybe I'll change the names of macros for rounding modes in v3.
Thank you for pointing that out.

Note that the rounding modes available in RISC-V is not exactly the same
as x86: the rounding mode "Round to nearest, ties to max magnitude"
doesn't exist in x86. Any ideas about naming the macro for that?

FYI, exception-related macros in different archs:
x86: _FPU_MASK_OM
m68k: _FPU_MASK_OVFL
mips: _FPU_MASK_O
aarch64: _FPU_FPCR_MASK_OFE
loongarch: _FPU_MASK_O
csky: _FPU_MASK_OFE

Also, I think it's inappropriate to name exception flags "_FPU_MASK_*M"
because instead of marking the bit as 1 for masking the exception,
RISC-V marks the bit as 1 for raising the exception, so I prefer
something like "_FPU_EXCEPT", "_FPU_RAISE", "_FPU_ACCRUE" or just
"_FPU_FLAG".

At 2023/2/14 2:17, Richard Henderson wrote:
> On 2/12/23 20:16, Shiqi Zhang wrote:
>>
>>> Honestly, no one should be using <fpu_control.h> at all.  It was
>>> originally an x86 specific thing.  The only reason to fill in this
>>> header at all is to match the original x86 API.  Therefore the symbol
>>> names should match ./sysdeps/x86/fpu_control.h.
>>
>>
>> Actually other architectures also defines these arch-specific macros in
>> ./sysdeps/***/fpu_control.h. And the symbol names doesn't match x86,
>> too. For example I checked the macro _FPU_FPCR_MASK_OFE in
>> ./sysdeps/aarch64/fpu/fpu_control.h, nothing in glibc referenced it so I
>> think it can only be for the users to control FPU in an arch-specific
>> manner.
>
> Oh the other hand, note the number of targets that use
>
> _FPU_RC_NEAREST, _FPU_RC_DOWN, _FPU_RC_UP, _FPU_RC_ZERO
>
> x86, alpha, csky, loongarch, m68k, mips, ppc, sparc, sh (nearest, zero).
>
> Indeed, arm and aarch64 seem to be the outliers in being completely
> different.
>
>
> r~