Re: [Qemu-ppc] [RFC PATCH v2 2/2] softfloat: Add round-to-odd rounding mode

[Peter Maydell]

On 27 January 2017 at 08:03, Bharata B Rao <address@hidden> wrote:
> Power ISA 3.0 introduces a few quadruple precision floating point
> instructions that support round-to-odd rounding mode. The
> round-to-odd mode is explained as under:
>
> Let Z be the intermediate arithmetic result or the operand of a convert
> operation. If Z can be represented exactly in the target format, the
> result is Z. Otherwise the result is either Z1 or Z2 whichever is odd.
> Here Z1 and Z2 are the next larger and smaller numbers representable
> in the target format respectively.
>
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
>  fpu/softfloat.c         | 17 ++++++++++++++++-
>  include/fpu/softfloat.h |  2 ++
>  2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index b04699c..1c322ad 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -623,6 +623,9 @@ static float64 roundAndPackFloat64(flag zSign, int zExp, 
> uint64_t zSig,
>      case float_round_down:
>          roundIncrement = zSign ? 0x3ff : 0;
>          break;
> +    case float_round_to_odd:
> +        roundIncrement = (zSig & 0x400) ? 0 : 0x3ff;
> +        break;
>      default:
>          abort();
>      }
> @@ -632,8 +635,10 @@ static float64 roundAndPackFloat64(flag zSign, int zExp, 
> uint64_t zSig,
>               || (    ( zExp == 0x7FD )
>                    && ( (int64_t) ( zSig + roundIncrement ) < 0 ) )
>             ) {
> +            bool overflow_to_inf = roundingMode != float_round_to_odd &&
> +                                   roundIncrement != 0;
>              float_raise(float_flag_overflow | float_flag_inexact, status);
> -            return packFloat64( zSign, 0x7FF, - ( roundIncrement == 0 ));
> +            return packFloat64( zSign, 0x7FF, - ( !overflow_to_inf ));

Can you follow the QEMU coding style, please? softfloat's coding
style is insane and we generally have been switching it to the
QEMU style as we touch lines of code.

>          }
>          if ( zExp < 0 ) {
>              if (status->flush_to_zero) {
> @@ -665,6 +670,9 @@ static float64 roundAndPackFloat64(flag zSign, int zExp, 
> uint64_t zSig,
>              case float_round_down:
>                  roundIncrement = zSign ? 0x3ff : 0;
>                  break;
> +            case float_round_to_odd:
> +                roundIncrement = (zSig & 0x400) ? 0 : 0x3ff;
> +                break;
>              default:
>                  abort();
>              }

Ah, this extra check does make a difference for the round-to-odd
case. But since it's a special for round-to-odd, better to
phrase as
  if (roundingMode == float_round_to_odd) {
      /* in this case roundIncrement depends on zSig, which just changed */
      roundIncrement = ...;
  }

rather than a case statement where most of the code is no-ops.

> @@ -1166,6 +1174,9 @@ static float128 roundAndPackFloat128(flag zSign, 
> int32_t zExp,
>      case float_round_down:
>          increment = zSign && zSig2;
>          break;
> +    case float_round_to_odd:
> +        increment = !(zSig1 & 0x1) && zSig2;
> +        break;
>      default:
>          abort();
>      }
> @@ -1185,6 +1196,7 @@ static float128 roundAndPackFloat128(flag zSign, 
> int32_t zExp,
>              if (    ( roundingMode == float_round_to_zero )
>                   || ( zSign && ( roundingMode == float_round_up ) )
>                   || ( ! zSign && ( roundingMode == float_round_down ) )
> +                 || ( roundingMode == float_round_to_odd )
>                 ) {
>                  return
>                      packFloat128(
> @@ -1232,6 +1244,9 @@ static float128 roundAndPackFloat128(flag zSign, 
> int32_t zExp,
>              case float_round_down:
>                  increment = zSign && zSig2;
>                  break;
> +            case float_round_to_odd:
> +                increment = !(zSig1 & 0x1) && zSig2;
> +                break;
>              default:
>                  abort();
>              }
> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index 842ec6b..8a39028 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -180,6 +180,8 @@ enum {
>      float_round_up           = 2,
>      float_round_to_zero      = 3,
>      float_round_ties_away    = 4,
> +    /* Not an IEEE rounding mode: round to the closest odd mantissa value */
> +    float_round_to_odd       = 5,
>  };
>
>  
> /*----------------------------------------------------------------------------
> --
> 2.7.4
>

Otherwise:
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM