On 31 December 2013 14:51, Richard Henderson <[email protected]> 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
