Re: [Qemu-devel] [RFC PATCH v0] softfloat: Add round-to-odd rounding mode

[Bharata B Rao]

On Thu, Jan 19, 2017 at 07:29:54AM -0800, Richard Henderson wrote:
> On 01/18/2017 09:14 PM, Bharata B Rao wrote:
> > Power ISA 3.0 introduces a few quadruple precision floating point
> > instructions that support round-to-add 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>
> > ---
> > - I am not fully sure if this the correct implementation for the above
> >   described round-to-odd rounding method. Any help is appreciated.
> > - Didn't bother to add round-to-odd to other floating point precision
> >   variants as round-to-odd option is currently supported only for some
> >   instructions that work on quad precision.
> > 
> >  fpu/softfloat.c         | 6 ++++++
> >  include/fpu/softfloat.h | 1 +
> >  2 files changed, 7 insertions(+)
> > 
> > diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> > index c295f31..05932a9 100644
> > --- a/fpu/softfloat.c
> > +++ b/fpu/softfloat.c
> > @@ -1149,6 +1149,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();
> >      }
> > @@ -1215,6 +1218,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();
> >              }
> 
> I believe you've missed the section in between that deals with
> round-to-largest or to infinity:
> 
>             if (    ( roundingMode == float_round_to_zero )
>                  || ( zSign && ( roundingMode == float_round_up ) )
>                  || ( ! zSign && ( roundingMode == float_round_down ) )
>                ) {

Addresed in v1. Thanks Richard and Peter for pointing this out.

> 
> The description in see in the manual on page 387 is more precise than what
> you quote above:

Right. I quoted from page 385 as it is more concise for patch
description. However if you think the more precise definition of page
387 is appropriate in commit, I can put it.

Regards,
Bharata.