Re: [PATCH 06/12] RISC-V: Generic <math.h> and soft-fp Routines

[Joseph Myers <joseph at codesourcery dot com>]

On Wed, 14 Jun 2017, Palmer Dabbelt wrote:

> diff --git a/sysdeps/riscv/bits/fenv.h b/sysdeps/riscv/bits/fenv.h

> +enum
> +  {
> +    FE_INEXACT = 0x01,
> +#define FE_INEXACT	FE_INEXACT
> +    FE_UNDERFLOW = 0x02,
> +#define FE_UNDERFLOW	FE_UNDERFLOW
> +    FE_OVERFLOW = 0x04,
> +#define FE_OVERFLOW	FE_OVERFLOW
> +    FE_DIVBYZERO = 0x08,
> +#define FE_DIVBYZERO	FE_DIVBYZERO
> +    FE_INVALID = 0x10,
> +#define FE_INVALID	FE_INVALID
> +  };

This clearly hasn't passed the glibc testsuite.  See how other bits/fenv.h 
headers now define macros to make them usable in #if (which the conform/ 
tests test for).

> diff --git a/sysdeps/riscv/bits/mathdef.h b/sysdeps/riscv/bits/mathdef.h

You shouldn't need this file any more.

> diff --git a/sysdeps/riscv/bits/mathinline.h b/sysdeps/riscv/bits/mathinline.h

> +/* Test for negative number.  Used in the signbit() macro.  */
> +__MATH_INLINE int
> +__NTH (__signbitf (float __x))
> +{
> +  return __builtin_signbitf (__x);
> +}
> +__MATH_INLINE int
> +__NTH (__signbit (double __x))
> +{
> +  return __builtin_signbit (__x);
> +}

Is this really of any use now, given that signbit uses __builtin_signbit 
directly and that the signbit macro, rather than __signbit etc. functions, 
is used for internal calls within glibc?

> +/* Leave it to the compiler to optimize these if __NO_MATH_ERRNO__.  */
> +# if defined __riscv_flen && !defined __NO_MATH_ERRNO__
> +
> +#  if __riscv_xlen >= 64
> +
> +__MATH_INLINE long long int
> +__NTH (llrintf (float __x))
> +{
> +  long long int __res;
> +  __asm__ __volatile__ ("fcvt.l.s %0, %1" : "=r" (__res) : "f" (__x));
> +  return __res;

This all looks rather like making inline functions replicate the bugs of 
out-of-line functions (bug 6797, 6798), i.e. a bad idea.

> diff --git a/sysdeps/riscv/ieee754.h b/sysdeps/riscv/ieee754.h

My reading of the ABI is that long double is 128-bit unconditionally for 
RISC-V.  Thus, you shouldn't need this header; the default 
sysdeps/ieee754/ldbl-128/ieee754.h should suffice.

To pass the math/ tests for soft-float, I'd expect you to need to have 
s_fma.c and s_fmaf.c that include the soft-fp versions, as on various 
other architectures.  (You also need a libm-test-ulps file or files, which 
appear to be missing from this patch series, to reflect the actual ulps 
seen on RISC-V.)

-- 
Joseph S. Myers
joseph@codesourcery.com