Re: [Qemu-devel] [PATCH 14/22] softfloat: Add support for ties-away rounding

[Peter Maydell]

On 31 December 2013 14:51, Richard Henderson <address@hidden> wrote:
> [Tom, this is exactly what you need to fix FRIN rounding.]
>
> On 12/31/2013 05:35 AM, Peter Maydell wrote:
>> -    float_round_to_zero      = 3
>> +    float_round_to_zero      = 3,
>> +    float_round_ties_away    = 4,
>
> I'm not keen on the name.  Does anyone else think float_round_nearest_inf is a
> better name?

The IEEE spec specifically calls this roundTiesToAway. I'd rather
not deviate from the official name unless there's a good reason.
(Though admittedly we don't really match up on the other rounding
mode names.)

>
>> +++ b/fpu/softfloat.c
>> @@ -107,7 +107,7 @@ static int32 roundAndPackInt32( flag zSign, uint64_t 
>> absZ STATUS_PARAM)
>>      roundingMode = STATUS(float_rounding_mode);
>>      roundNearestEven = ( roundingMode == float_round_nearest_even );
>>      roundIncrement = 0x40;
>> -    if ( ! roundNearestEven ) {
>> +    if (!roundNearestEven && roundingMode != float_round_ties_away) {
>>          if ( roundingMode == float_round_to_zero ) {
>>              roundIncrement = 0;
>>          }
>
> This whole section of code is now a mess.  I know you're looking for minimal
> changes here, but perhaps I can convince you that
>
>     switch (roundingMode) {
>     case float_round_nearest_even:
>     case float_round_ties_away:
>         roundIncrement = 0x40;
>         break;
>     case float_round_to_zero:
>         roundIncrement = 0;
>         break;
>     case float_round_up:
>         roundIncrement = zSign ? 0 : 0x7f;
>         break;
>     case float_round_down:
>         roundIncrement = zSign ? 0x7f : 0;
>         break;
>     default:
>         abort();
>     }
>
> is easier to follow?

I agree it looks much better. I'd prefer to keep adding features
and refactoring code in separate patches, though.

thanks
-- PMM