[Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().

[Carl Worth]

On Thu, Mar 12 2015, Matt Turner wrote:
> +/* The C standard library has functions round()/rint()/nearbyint() that round
> + * their arguments according to the rounding mode set in the floating-point
> + * control register. While there are trunc()/ceil()/floor() functions that do
> + * a specific operation without modifying the rounding mode, there is no
> + * roundeven() in any version of C.
> + *
> + * Technical Specification 18661 (ISO/IEC TS 18661-1:2014) adds roundeven(),
> + * but it's unfortunately not implemented by glibc.
> + *
> + * This implementation differs in that it does not raise the inexact exception.
> + */

This documentation needs the actual description of the behavior of the
function. Most of the above comment is commentary on the implementation
itself.

Perhaps something like the following?

	Round \x to the nearest even integer (returned in floating-point
	format).

	Note: This function is similar to roundeven() as specified by
	Technical Specification 18661 (ISO/IEC TS 18661-1:2014),
	differing only in that _mesa_roundeven() won't necessarily raise
	the inexact exception as roundeven() would.

Then, (within the function body itself?), it makes sense to have a
comment on the implementation:

	When glibc eventually implements the standardized roundeven() we
	could use it directly. Until then, the available C standard
	library functions have the following limitations:

		round()/rint()/nearbyint(): Behavior varies depending on
		the rounding mode set in the floating-point control
		register.

		trunc()/ceil()/floor(): These specify rounding without
		relying on the rounding mode, but none give the rounding
		behavior desired for _mesa_roundeven().

	<But since none of those work??? See my questions below...>

> +static inline float
> +_mesa_roundevenf(float x)
> +{
> +   float ret;
> +   /* Assume that the floating-point rounding mode has not been changed from
> +    * the default (Round to nearest).
> +    */
> +   ret = rintf(x);
> +   return ret;
> +}

But after all that commentary about how rint() doesn't provide what's
needed, here the implementation is just using it anyway?

The comment here, (and even any history of "other parts of mesa make the
same assumption"), doesn't give the code adequate robustness. So at
least an assert is needed here. Is this correct: ?

	assert (fegetround() == FE_TONEAREST);

But beyond that, I'm actually confused---how can the default rounding
mode (rounding toward nearest number, right?) give us an adequate
implementation for roundeven()?

[Thanks for sending the split patch. It made this question very
apparent, where I wasn't asking it about the combined series. Thanks
also for adding the test and the SSE optimization as a separate
commit. Those look good to me.]

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150312/fe9762ed/attachment.sig>