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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 31 December 2013 14:51, Richard Henderson <rth@xxxxxxxxxxx> 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
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm




[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux