Patchwork [libgcc,ARM] __gnu_f2h_internal inaccuracy

login
register
mail settings
Submitter John Tytgat
Date Dec. 11, 2012, 10:04 p.m.
Message ID <585499fc52.Jo@hobbes.bass-software.com>
Download mbox | patch
Permalink /patch/205315/
State New
Headers show

Comments

John Tytgat - Dec. 11, 2012, 10:04 p.m.
Ping ? Paul, seen that you've contributed fp16.c together with Sandra
Loosemore, perhaps you can review this patch please ?

John.

In message <ab11eef452.Jo@hobbes.bass-software.com>
          John Tytgat <john@bass-software.com> wrote:

> __gnu_f2h_internal in libgcc converts single-precision floating-point value
> to half-precision value for ARM.  I noticed there is a slight inaccuracy
> for floating-point values 2^-25 + epsilon (with epsilon > 0 and < 2^-26).
> Those should all be converted to 2^-24 in half-precision.
> 
> And this because the mask used to implement the even-odd rounding for
> aexp = -25 is wrong.  Currently we have:
> 
>   /* Decimal point between bits 22 and 23.  */
>   mantissa |= 0x00800000;
>   if (aexp < -14)
>     {
>       mask = 0x007fffff;
>       if (aexp < -25)
>         aexp = -26;
>       else if (aexp != -25)
>         mask >>= 24 + aexp;
>     }
>   else
>     mask = 0x00001fff;
> 
> But when aexp is 25 the mask should be 0xffffff instead of 0x7fffff as the
> decimal dot in half-precision will be between bit 24 and 23 of the
> above mentioned mantissa.  Cfr. the even-odd rounding done:
> 
>   /* Round.  */
>   if (mantissa & mask)
>     {
>       increment = (mask + 1) >> 1;
>       if ((mantissa & mask) == increment)
>         increment = mantissa & (increment << 1);
>       mantissa += increment;
>       if (mantissa >= 0x01000000)
>         {
>           mantissa >>= 1;
>           aexp++;
>         }
>     }
> 
> Attached patch solves this problem.  I've left out the clamping of
> aexp to -26 for values less than -25 as this it not necessary.  After
> the even-odd rounding all aexp values less than -24 will result in +0. or
> -0. anyway.
> 
> John Tytgat  <John@bass-software.com>
> 
> 	* config/arm/fp16.c (__gnu_f2h_internal): Fix inaccuracy.
> 
> I've got a copyright assignment but no write access.
> 
> John Tytgat.
Nick Clifton - Dec. 13, 2012, 12:03 p.m.
Hi John,

> John Tytgat<John@bass-software.com>
>>
>>	* config/arm/fp16.c (__gnu_f2h_internal): Fix inaccuracy.
>>

Approved and applied.

Cheers
   Nick

Patch

Index: libgcc/config/arm/fp16.c
===================================================================
--- libgcc/config/arm/fp16.c	(revision 193830)
+++ libgcc/config/arm/fp16.c	(working copy)
@@ -47,11 +47,9 @@ 
   mantissa |= 0x00800000;
   if (aexp < -14)
     {
-      mask = 0x007fffff;
-      if (aexp < -25)
-	aexp = -26;
-      else if (aexp != -25)
-	mask >>= 24 + aexp;
+      mask = 0x00ffffff;
+      if (aexp >= -25)
+        mask >>= 25 + aexp;
     }
   else
     mask = 0x00001fff;