Re: [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Thu, 3 Jul 2014, Adhemerval Zanella wrote:

> +  /* Generates the equivalent of feclearexcept (FE_ALL_EXCEPT):
> +
> +     double fenv_clear = __builtin_mffs ();
> +     *(uint64_t)&fenv_clear &= 0xffffffff00000000LL;
> +     __builtin_mtfsf (0xff, fenv_clear);  */
> +
> +  /* Mask to clear everything except for the rounding modes and non-IEEE
> +     arithmetic flag.  */
> +  const unsigned HOST_WIDE_INT clear_exception_mask =
> +    HOST_WIDE_INT_UC (0xffffffff00000000);

This mask is different from the one before, and it looks like it's 
clearing the rounding mode bits.

You probably don't need to do this masking here.  In general, for the 
feclearexcept operation it's sufficient to reuse the same status/control 
register settings as you used in the feholdexcept operation - nothing 
(visible at C level) should have changed since that call except for the 
exception flags, and anyway when the feclearexcept operation is executed, 
the logical idea is to make things as if the floating-point operation 
preceding the failed compare-and-exchange never happened, so reusing the 
register setting makes logical sense in that way as well.  (On x86, that 
reuse is what's done for SSE floating point; for 387 we use fnclex in both 
operations, and never explicitly compute a control word setting with 
exceptions cleared and masked.)

Other than that I don't see any issues with the changes (this is not an 
approval of the patch, however).

The testsuite changes are OK.

-- 
Joseph S. Myers
joseph@codesourcery.com