public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, libgcc, ARM] __gnu_f2h_internal inaccuracy
@ 2012-11-27  0:41 John Tytgat
  2012-11-27  0:47 ` John Tytgat
  2012-12-11 22:05 ` John Tytgat
  0 siblings, 2 replies; 4+ messages in thread
From: John Tytgat @ 2012-11-27  0:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: ian, nickc, richard.earnshaw, paul, ramana.radhakrishnan

__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.
-- 
John Tytgat                                                             BASS
John@bass-software.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, libgcc, ARM] __gnu_f2h_internal inaccuracy
  2012-11-27  0:41 [PATCH, libgcc, ARM] __gnu_f2h_internal inaccuracy John Tytgat
@ 2012-11-27  0:47 ` John Tytgat
  2012-12-11 22:05 ` John Tytgat
  1 sibling, 0 replies; 4+ messages in thread
From: John Tytgat @ 2012-11-27  0:47 UTC (permalink / raw)
  To: gcc-patches, ian, nickc, richard.earnshaw, paul, ramana.radhakrishnan

[-- Attachment #1: Type: text/plain, Size: 257 bytes --]

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

> [...] Attached patch solves this problem. [...]

This time for real.

John Tytgat.
-- 
John Tytgat, in his comfy chair at home
John.Tytgat@aaug.net

[-- Attachment #2: gnu_f2h_internal.diff --]
[-- Type: text/plain, Size: 514 bytes --]

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;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, libgcc, ARM] __gnu_f2h_internal inaccuracy
  2012-11-27  0:41 [PATCH, libgcc, ARM] __gnu_f2h_internal inaccuracy John Tytgat
  2012-11-27  0:47 ` John Tytgat
@ 2012-12-11 22:05 ` John Tytgat
  2012-12-13 12:06   ` nick clifton
  1 sibling, 1 reply; 4+ messages in thread
From: John Tytgat @ 2012-12-11 22:05 UTC (permalink / raw)
  To: gcc-patches, paul; +Cc: ian, nickc, richard.earnshaw, ramana.radhakrishnan

[-- Attachment #1: Type: text/plain, Size: 1994 bytes --]

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.

-- 
John Tytgat                                                             BASS
John@bass-software.com

[-- Attachment #2: gnu_f2h_internal.diff --]
[-- Type: text/plain, Size: 516 bytes --]

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;


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, libgcc, ARM] __gnu_f2h_internal inaccuracy
  2012-12-11 22:05 ` John Tytgat
@ 2012-12-13 12:06   ` nick clifton
  0 siblings, 0 replies; 4+ messages in thread
From: nick clifton @ 2012-12-13 12:06 UTC (permalink / raw)
  To: John Tytgat
  Cc: gcc-patches, paul, ian, richard.earnshaw, ramana.radhakrishnan

Hi John,

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

Approved and applied.

Cheers
   Nick

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-12-13 12:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-27  0:41 [PATCH, libgcc, ARM] __gnu_f2h_internal inaccuracy John Tytgat
2012-11-27  0:47 ` John Tytgat
2012-12-11 22:05 ` John Tytgat
2012-12-13 12:06   ` nick clifton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).