Re: [PATCH 04/28] powerpc: ceil/ceilf refactor

["Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>]

On Fri, Mar 29 2019, Adhemerval Zanella wrote:
> 
> The IFUNC organization for powerpc64 is also change to be enabled only
> for powerpc64 and not for powerpc64le.

OK.

> 	* sysdeps/powerpc/fpu/fenv_libc.h (__fesetround_inline_nocheck): New
> 	macro.

I really wish we can get rid of the burden of writing changelogs, but
for now shouldn't this be function, instead of macro?

> +/* Same as __fesetround_inline, however without runtime check to use DFP
> +   mtfsfi syntax or if round is valid.  */

What would be a runtime check to use DFP syntax? Are you referring to
the optional third parameter (W) to mtfsfi?  I don't see such mechanism
in __fesetround_inline, so I'm not sure I understand this
comment/comparison.

> +static inline void
> +__fesetround_inline_nocheck (const int round)
> +{
> +  asm volatile ("mtfsfi 7,%0" : : "i" (round));
> +}
> +

The implementation looks OK.

> +enum round_mode
> +{
> +  CEIL,
> +  FLOOR,
> +  ROUND,
> +  TRUNC,
> +  NEARBYINT,
> +};

OK.  To be used in following patches.

> +static inline void
> +set_fenv_mode (enum round_mode mode)
> +{
> +  int rmode;
> +  switch (mode)
> +  {
> +  case CEIL:  rmode = FE_UPWARD; break;
> +  default:    rmode = FE_TONEAREST; break;
> +  }
> +  __fesetround_inline_nocheck (rmode);
> +}

OK.

> +static inline float
> +round_to_integer_float (enum round_mode mode, float x)
> +{
> +  /* Ensure sNaN input is converted to qNaN.  */
> +  if (__glibc_unlikely (isnan (x)))
> +    return x + x;
> +
> +  if (fabs (x) > 0x1p+23)
> +    return x;
> +
> +  float r = x;
> +
> +  /* Save current FPU rounding mode and inexact state.  */
> +  fenv_t fe = fegetenv_register ();
> +  set_fenv_mode (mode);
> +  if (x > 0.0)
> +    {
> +      r += 0x1p+23;
> +      r -= 0x1p+23;
> +      r = fabs (r);
> +    }
> +  else if (x < 0.0)
> +    {
> +      r -= 0x1p+23;
> +      r += 0x1p+23;
> +      r = -fabs (r);
> +    }
> +  __builtin_mtfsf (0xff, fe);
> +
> +  return r;
> +}

Ok.

> +static inline double
> +round_to_integer_double (enum round_mode mode, double x)
> +{
> +  /* Ensure sNaN input is converted to qNaN.  */
> +  if (__glibc_unlikely (isnan (x)))
> +    return x + x;
> +
> +  if (fabs (x) > 0x1p+52)
> +    return x;
> +
> +  double r = x;
> +
> +  /* Save current FPU rounding mode and inexact state.  */
> +  fenv_t fe = fegetenv_register ();
> +  set_fenv_mode (mode);
> +  if (x > 0.0)
> +    {
> +      r += 0x1p+52;
> +      r -= 0x1p+52;
> +      r = fabs (r);
> +    }
> +  else if (x < 0.0)
> +    {
> +      r -= 0x1p+52;
> +      r += 0x1p+52;
> +      r = -fabs (r);
> +    }
> +  __builtin_mtfsf (0xff, fe);
> +
> +  return r;
> +}

OK.

> +double
> +__ceil (double x)
> +{
> +#ifdef _ARCH_PWR5X
> +  return __builtin_ceil (x);
> +#else
> +  return round_to_integer_double (CEIL, x);
> +#endif

The arch check looks correct.