diff mbox

[RFC,v0] softfloat: Add round-to-odd rounding mode

Message ID 1484802878-22681-1-git-send-email-bharata@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bharata B Rao Jan. 19, 2017, 5:14 a.m. UTC
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 <bharata@linux.vnet.ibm.com>
---
- 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(+)

Comments

Peter Maydell Jan. 19, 2017, 12:11 p.m. UTC | #1
On 19 January 2017 at 05:14, Bharata B Rao <bharata@linux.vnet.ibm.com> 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 <bharata@linux.vnet.ibm.com>
> ---
> - 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.

ARM has a couple of instructions that also want round-to-odd,
specifically for 32-bit results. So we should probably extend
it to those at some point (at the moment the target/arm code just
treats it like nearest-even, so gets inaccurate results).

>  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;

I think this is the right calculation, yes.
The ARM spec expresses the rounding operation in terms
of "force the LS bit of the mantissa to 1", but I think
doing it by setting increment like this helps us get
the corner cases right (like tininess detection).

The remaining part that your patch doesn't address is
the handling of rounding to infinity if the precise
result is too large to fit in the float128.
For ARM this is clearly defined in the spec: the
rounding pseudocode doesn't permit overflow-to-infinity.
This means the condition
            if (    ( roundingMode == float_round_to_zero )
                 || ( zSign && ( roundingMode == float_round_up ) )
                 || ( ! zSign && ( roundingMode == float_round_down ) )
which controls "do we return the maximum-float-value?"
needs an extra (roundingMode == float_round_to_odd) clause.

Hopefully those semantics work for the PPC case too?

>      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();
>              }
> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index 842ec6b..1463062 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -180,6 +180,7 @@ enum {
>      float_round_up           = 2,
>      float_round_to_zero      = 3,
>      float_round_ties_away    = 4,
> +    float_round_to_odd       = 5,

Maybe worth a comment:
    /* Not an IEEE rounding mode: round to the closest odd mantissa value */

since all our other rounding modes are as defined in the IEEE spec.

>  };
>
>  /*----------------------------------------------------------------------------
> --

thanks
-- PMM
Eric Blake Jan. 19, 2017, 2:47 p.m. UTC | #2
On 01/18/2017 11: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

s/add/odd/

> 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.

IEEE 854 and thus POSIX <float.h> specifies round-to-zero,
round-to-pos-inf, round-to-neg-inf, and round-to-even. It sounds like
round-to-odd is similar to round-to-even, only that the ties are broken
in the opposite direction.  I don't know if portable C code can even
request this mode (although float.h is allowed to add additional
implementation-defined values to FLT_ROUNDS), but since the hardware can
do it, I guess we have to emulate it.

> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
> - I am not fully sure if this the correct implementation for the above
>   described round-to-odd rounding method. Any help is appreciated.

I don't know the code either, but if round-to-even is implemented
correctly, it sounds like round-to-odd is a one-off tweak from that.
Peter Maydell Jan. 19, 2017, 3:20 p.m. UTC | #3
On 19 January 2017 at 14:47, Eric Blake <eblake@redhat.com> wrote:
> On 01/18/2017 11:14 PM, Bharata B Rao wrote:
>> 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.
>
> IEEE 854 and thus POSIX <float.h> specifies round-to-zero,
> round-to-pos-inf, round-to-neg-inf, and round-to-even. It sounds like
> round-to-odd is similar to round-to-even, only that the ties are broken
> in the opposite direction.

No, round-to-even does "round ties to even" -- the rounding is
to the closest representable number, and only if the two
surrounding numbers are both equally near do we pick the even
one. round-to-odd is "always round to the odd number, even
if that's much further from the infinitely-precise result
than the even number is". (This is also sometimes called
von Neumann rounding or sticky rounding -- the motivation is
that you can for instance do 64bit -> 32bit -> 16bit
conversions in two steps without double rounding errors if
you do the first conversion with round-to-odd.)

thanks
-- PMM
Richard Henderson Jan. 19, 2017, 3:29 p.m. UTC | #4
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 <bharata@linux.vnet.ibm.com>
> ---
> - 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 ) )
                ) {

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

   # If IR is exact, choose IR.
   # Otherwise choose NL, and if Guard=1, Round=1 or Sticky=1,
   # the least significant bit of the result is set to 1.

The "choose NL" part means round-to-zero, which means that we do not overflow 
to infinity.  And since FLOAT128_MAX is already odd, we're done.

Otherwise this looks ok.


r~
Peter Maydell Jan. 19, 2017, 3:44 p.m. UTC | #5
On 19 January 2017 at 15:29, Richard Henderson <rth@twiddle.net> wrote:
> The description in see in the manual on page 387 is more precise than what
> you quote above:
>
>   # If IR is exact, choose IR.
>   # Otherwise choose NL, and if Guard=1, Round=1 or Sticky=1,
>   # the least significant bit of the result is set to 1.
>
> The "choose NL" part means round-to-zero, which means that we do not
> overflow to infinity.  And since FLOAT128_MAX is already odd, we're done.

...so it does have the same semantics as ARM. Good :-)

thanks
-- PMM
Bharata B Rao Jan. 20, 2017, 9:20 a.m. UTC | #6
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 <bharata@linux.vnet.ibm.com>
> > ---
> > - 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.
diff mbox

Patch

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();
             }
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 842ec6b..1463062 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -180,6 +180,7 @@  enum {
     float_round_up           = 2,
     float_round_to_zero      = 3,
     float_round_ties_away    = 4,
+    float_round_to_odd       = 5,
 };
 
 /*----------------------------------------------------------------------------