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

[Matt Turner]

On Thu, Mar 12, 2015 at 2:59 PM, Carl Worth <cworth at cworth.org> wrote:
> 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.

Sounds good.

> 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().

I don't know why we would want to document that these functions don't
implement the behavior we want. That seems obvious enough...

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

I'm not sure what you mean. My intention in noting the existence of
trunc/ceil/floor is to note that there isn't an analogous function
with round-to-even behavior -- we have to use rint() with a particular
rounding mode.

>> +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?

I think you misread. rint() *does* provide the behavior we want
(round-to-nearest, half to even) when the rounding mode is the default
round-to-nearest.

As Eric noted in his original patch, the man pages for
rint(3)/nearbyint(3) don't describe the half-to-even behavior but
round(3) does:

> These  functions  round  x  to  the  nearest  integer, but round halfway cases away from zero (regardless of the current rounding direction, see fenv(3)), instead of to the nearest even integer like rint(3).

Maybe I should send a patch to the Linux man-pages project too.

> 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);

I don't really want to do this. We rely on the default rounding mode
everywhere. I don't think asserting here is useful. Also it requires
adding code for MSVC since it doesn't have fegetround().

> 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()?

Answered above.