This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] powerpc: fix check-before-set in SET_RESTORE_ROUND


Paul Clarke <pc@us.ibm.com> writes:

> A performance regression was introduced by commit
> 84d74e427a771906830800e574a72f8d25a954b8 "powerpc: Cleanup fenv_private.h".
>
> In the powerpc implementation of SET_RESTORE_ROUND, there is the
> following code in the "SET" function (slightly simplified):
> --
>   old.fenv = fegetenv_register ();
>
>   new.l = (old.l & _FPU_MASK_TRAPS_RN) | r; (1)
>
>   if (new.l != old.l)                       (2)
>     {
>       if ((old.l & _FPU_ALL_TRAPS) != 0)
>         (void) __fe_mask_env ();
>       fesetenv_register (new.fenv);         (3)
> --
>
> Line (1) sets the value of "new" to the current value of FPSCR,
> but masks off summary bits, exceptions, non-IEEE mode, and
> rounding mode, then ORs in the new rounding mode.
>
> Line (2) compares this new value to the current value in order to
> avoid setting a new value in the FPSCR (line (3)) unless something
> significant has changed (exception enables or rounding mode).
>
> The summary bits are not germane to the comparison, but are cleared
> in "new" and preserved in "old", resulting in false negative
> comparisons, and unnecessarily setting the FPSCR in those cases
> with associated negative performance impacts.
>
> The solution is to treat the summaries identically for "new" and "old":
> - save them in SET
> - leave them alone otherwise
> - restore the saved values in RESTORE
>
> Also minor changes:
> - expand _FPU_MASK_RN to 64bit hex, to match other MASKs
> - treat bit 52 (left-to-right) as reserved (since it is)
>
> 2017-10-10  Paul A. Clarke  <pc@us.ibm.com>
>
> 	* sysdeps/powerpc/fpu/fenv_private.h: Fix masks to
> 	more properly handle summary bits.

I think this ChangeLog entry has to mention all the macros that are being
changed, e.g.:

	* sysdeps/powerpc/fpu/fenv_private.h (_FPU_MASK_TRAPS_RN):
	(_FPU_MASK_FRAC_INEX_RET_CC): Fix masks to more properly handle
	summary bits.
	(_FPU_MASK_RN): Expand _FPU_MASK_RN to 64bit hex.
	(_FPU_MASK_NOT_RN_NI): Treat bit 52 (left-to-right) as reserved. 

Reviewed-by: Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>

-- 
Tulio Magno


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]