Message ID | 20230212065214.2399-1-shiqi@isrc.iscas.ac.cn |
---|---|
State | New |
Headers | show |
Series | [v2] riscv: Add macros for FPUCW/fcsr in fpu_control.h | expand |
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~
> 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.
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.
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. >
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~
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~
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.
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__)));