public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RTABI half-precision conversion functions (ping)
@ 2012-07-19 12:28 Julian Brown
  2012-07-19 12:55 ` Paul Brook
  0 siblings, 1 reply; 5+ messages in thread
From: Julian Brown @ 2012-07-19 12:28 UTC (permalink / raw)
  To: gcc-patches; +Cc: ams, ramana.radhakrishnan, rearnsha, paul

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

Hi,

The patch that Andrew Stubbs sent upstream here:

http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02130.html

seems to have become stalled after Ramana's question here:

http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00291.html

This was discussed with ARM personnel (i.e. Lee Smith) at the time the
patch was created. AFAICT, our reasoning went like this (Paul, please
correct me if I'm just making stuff up): we wish to use aliases for the
old and new names for the helpers. The existing
__gnu_{f2h,h2f}_{ieee,alternative} helpers take unsigned short
arguments, and therefore assume (on function entry) that the values
contained in those arguments are zero-extended (although, due to the
vagaries of libcall handling in GCC, that may not actually be true --
at the time, this was a latent bug, and may in fact still be). The
__aeabi_* helpers are defined to pass/return unadorned "short" values,
intending to convey that only the low-order 16 bits of half-float
values contain meaningful data.

But, that means EABI-conformant callers are also perfectly entitled to
sign-extend half-float values before calling our helper functions
(although GCC itself won't do that). Using "unsigned int" and taking
care to only examine the low-order bits of the value in the helper
function itself serves to fix the latent bug, is compatible with
existing code, allows us to be conformant with the eabi, and allows use
of aliases to make the __gnu and __aeabi functions the same.

The patch no longer applied as-is, so I've updated it (attached,
re-tested). Note that there are no longer any target-independent changes
(though I'm not certain that the symbol versions are still correct).

OK to apply?

Julian

ChangeLog

    Andrew Stubbs  <ams@codesourcery.com>
    Julian Brown  <julian@codesourcery.com>

    gcc/
    * config/arm/arm.c (arm_init_libfuncs): Change __gnu_f2h_ieee to
    __aeabi_f2h, __gnu_f2h_alternative to __aeabi_f2h_alt,
    __gnu_h2f_ieee to __aeabi_h2f, and __gnu_h2f_alternative to
    __aeabi_h2f_alt.

    libgcc/
    * config/arm/fp16.c (__gnu_f2h_internal): Change return type to
    unsigned int. Change 'sign' variable likewise.
    (__gnu_h2f_internal): Set to static inline.
    Change return type to unsigned int. Change 'sign' variable
    likewise. (ALIAS): New define.
    (__gnu_f2h_ieee): Change unsigned short to unsigned int.
    (__gnu_h2f_ieee): Likewise.
    (__gnu_f2h_alternative): Likewise.
    (__gnu_h2f_alternative): Likewise.
    (__aeabi_f2h, __aeabi_h2f): New aliases.
    (__aeabi_f2h_alt, __aeabi_h2f_alt): Likewise.
    * config/arm/libgcc-bpabi.ver (__aeabi_f2h, __aeabi_f2h_alt)
    (__aeabi_h2f, __aeabi_h2f_alt): Set versions.
    * config/arm/sfp-machine.h (__extendhfsf2): Set to __aeabi_h2f.
    (__truncsfhf2): Set to __aeabi_f2h.
    * config/arm/t-bpabi (LIB2ADD_ST): Move fp16.c ...
    (LIB2ADD): ... to here.
    * config/arm/t-symbian (LIB2ADD_ST): Move fp16.c ...
    (LIB2ADD): ... to here.

    gcc/testsuite/
    * g++.dg/ext/arm-fp16/arm-fp16-ops-5.C: Check for __aeabi_h2f
    and __aeabi_f2h.
    * g++.dg/ext/arm-fp16/arm-fp16-ops-6.C: Likewise.
    * gcc.dg/torture/arm-fp16-ops-5.c: Likewise.
    * gcc.dg/torture/arm-fp16-ops-6.c: Likewise.


[-- Attachment #2: eabi-half-prec-fn-names-1.diff --]
[-- Type: text/x-patch, Size: 7111 bytes --]

commit 03ffa771d52daeee8bf757bcdc2d982aa458aebd
Author: Julian Brown <jbrown@mentor.com>
Date:   Wed Jul 18 08:43:12 2012 -0700

    EABI half-precision helper function names.

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index ff46dd9..c22c2b7 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -1238,12 +1238,12 @@ arm_init_libfuncs (void)
       /* Conversions.  */
       set_conv_libfunc (trunc_optab, HFmode, SFmode,
 			(arm_fp16_format == ARM_FP16_FORMAT_IEEE
-			 ? "__gnu_f2h_ieee"
-			 : "__gnu_f2h_alternative"));
+			 ? "__aeabi_f2h"
+			 : "__aeabi_f2h_alt"));
       set_conv_libfunc (sext_optab, SFmode, HFmode,
 			(arm_fp16_format == ARM_FP16_FORMAT_IEEE
-			 ? "__gnu_h2f_ieee"
-			 : "__gnu_h2f_alternative"));
+			 ? "__aeabi_h2f"
+			 : "__aeabi_h2f_alt"));
 
       /* Arithmetic.  */
       set_optab_libfunc (add_optab, HFmode, NULL);
diff --git a/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-5.C b/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-5.C
index 92bc8a9..738d26d 100644
--- a/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-5.C
+++ b/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-5.C
@@ -13,3 +13,5 @@
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h\[a-z\]*_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h2f_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_f2h_ieee" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_h2f" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_f2h" } } */
diff --git a/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-6.C b/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-6.C
index ae40b1e..a0e09c8 100644
--- a/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-6.C
+++ b/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-6.C
@@ -13,3 +13,5 @@
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h\[a-z\]*_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h2f_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_f2h_ieee" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_h2f" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_f2h" } } */
diff --git a/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-5.c b/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-5.c
index 92bc8a9..738d26d 100644
--- a/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-5.c
+++ b/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-5.c
@@ -13,3 +13,5 @@
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h\[a-z\]*_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h2f_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_f2h_ieee" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_h2f" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_f2h" } } */
diff --git a/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-6.c b/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-6.c
index ae40b1e..a0e09c8 100644
--- a/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-6.c
+++ b/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-6.c
@@ -13,3 +13,5 @@
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h\[a-z\]*_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h2f_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_f2h_ieee" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_h2f" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_f2h" } } */
diff --git a/libgcc/config/arm/fp16.c b/libgcc/config/arm/fp16.c
index 936caeb..a49401d 100644
--- a/libgcc/config/arm/fp16.c
+++ b/libgcc/config/arm/fp16.c
@@ -22,10 +22,10 @@
    see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
    <http://www.gnu.org/licenses/>.  */
 
-static inline unsigned short
+static inline unsigned int
 __gnu_f2h_internal(unsigned int a, int ieee)
 {
-  unsigned short sign = (a >> 16) & 0x8000;
+  unsigned int sign = (a >> 16) & 0x8000;
   int aexp = (a >> 23) & 0xff;
   unsigned int mantissa = a & 0x007fffff;
   unsigned int mask;
@@ -95,10 +95,10 @@ __gnu_f2h_internal(unsigned int a, int ieee)
   return sign | (((aexp + 14) << 10) + (mantissa >> 13));
 }
 
-unsigned int
-__gnu_h2f_internal(unsigned short a, int ieee)
+static inline unsigned int
+__gnu_h2f_internal(unsigned int a, int ieee)
 {
-  unsigned int sign = (unsigned int)(a & 0x8000) << 16;
+  unsigned int sign = (a & 0x00008000) << 16;
   int aexp = (a >> 10) & 0x1f;
   unsigned int mantissa = a & 0x3ff;
 
@@ -120,26 +120,33 @@ __gnu_h2f_internal(unsigned short a, int ieee)
   return sign | (((aexp + 0x70) << 23) + (mantissa << 13));
 }
 
-unsigned short
+#define ALIAS(src, dst) \
+  typeof (src) dst __attribute__ ((alias (#src)));
+
+unsigned int
 __gnu_f2h_ieee(unsigned int a)
 {
   return __gnu_f2h_internal(a, 1);
 }
+ALIAS (__gnu_f2h_ieee, __aeabi_f2h)
 
 unsigned int
-__gnu_h2f_ieee(unsigned short a)
+__gnu_h2f_ieee(unsigned int a)
 {
   return __gnu_h2f_internal(a, 1);
 }
+ALIAS (__gnu_h2f_ieee, __aeabi_h2f)
 
-unsigned short
+unsigned int
 __gnu_f2h_alternative(unsigned int x)
 {
   return __gnu_f2h_internal(x, 0);
 }
+ALIAS (__gnu_f2h_alternative, __aeabi_f2h_alt)
 
 unsigned int
-__gnu_h2f_alternative(unsigned short a)
+__gnu_h2f_alternative(unsigned int a)
 {
   return __gnu_h2f_internal(a, 0);
 }
+ALIAS (__gnu_h2f_alternative, __aeabi_h2f_alt)
diff --git a/libgcc/config/arm/libgcc-bpabi.ver b/libgcc/config/arm/libgcc-bpabi.ver
index 3ba8364..5bb5f04 100644
--- a/libgcc/config/arm/libgcc-bpabi.ver
+++ b/libgcc/config/arm/libgcc-bpabi.ver
@@ -106,3 +106,10 @@ GCC_3.5 {
 GCC_4.3.0 {
   _Unwind_Backtrace
 }
+
+GCC_4.7.0 {
+  __aeabi_f2h
+  __aeabi_f2h_alt
+  __aeabi_h2f
+  __aeabi_h2f_alt
+}
diff --git a/libgcc/config/arm/sfp-machine.h b/libgcc/config/arm/sfp-machine.h
index a89d05a..f2d7a37 100644
--- a/libgcc/config/arm/sfp-machine.h
+++ b/libgcc/config/arm/sfp-machine.h
@@ -99,7 +99,7 @@ typedef int __gcc_CMPtype __attribute__ ((mode (__libgcc_cmp_return__)));
 #define __fixdfdi	__aeabi_d2lz
 #define __fixunsdfdi	__aeabi_d2ulz
 #define __floatdidf	__aeabi_l2d
-#define __extendhfsf2	__gnu_h2f_ieee
-#define __truncsfhf2	__gnu_f2h_ieee
+#define __extendhfsf2	__aeabi_h2f
+#define __truncsfhf2	__aeabi_f2h
 
 #endif /* __ARM_EABI__ */
diff --git a/libgcc/config/arm/t-bpabi b/libgcc/config/arm/t-bpabi
index e79cbd7..adf977a 100644
--- a/libgcc/config/arm/t-bpabi
+++ b/libgcc/config/arm/t-bpabi
@@ -3,9 +3,8 @@ LIB1ASMFUNCS += _aeabi_lcmp _aeabi_ulcmp _aeabi_ldivmod _aeabi_uldivmod
 
 # Add the BPABI C functions.
 LIB2ADD += $(srcdir)/config/arm/bpabi.c \
-	   $(srcdir)/config/arm/unaligned-funcs.c
-
-LIB2ADD_ST += $(srcdir)/config/arm/fp16.c
+	   $(srcdir)/config/arm/unaligned-funcs.c \
+	   $(srcdir)/config/arm/fp16.c
 
 LIB2ADDEH = $(srcdir)/config/arm/unwind-arm.c \
   $(srcdir)/config/arm/libunwind.S \
diff --git a/libgcc/config/arm/t-symbian b/libgcc/config/arm/t-symbian
index d573157..248bf78 100644
--- a/libgcc/config/arm/t-symbian
+++ b/libgcc/config/arm/t-symbian
@@ -13,7 +13,7 @@ LIB1ASMFUNCS += \
 	_fixsfsi _fixunssfsi
 
 # Include half-float helpers.
-LIB2ADD_ST += $(srcdir)/config/arm/fp16.c
+LIB2ADD += $(srcdir)/config/arm/fp16.c
 
 # Include the gcc personality routine
 LIB2ADDEH = $(srcdir)/unwind-c.c $(srcdir)/config/arm/pr-support.c

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

* Re: RTABI half-precision conversion functions (ping)
  2012-07-19 12:28 RTABI half-precision conversion functions (ping) Julian Brown
@ 2012-07-19 12:55 ` Paul Brook
  2012-07-19 13:48   ` Julian Brown
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Brook @ 2012-07-19 12:55 UTC (permalink / raw)
  To: Julian Brown; +Cc: gcc-patches, ams, ramana.radhakrishnan, rearnsha

> Hi,
> 
> The patch that Andrew Stubbs sent upstream here:
> 
> http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02130.html
> 
> seems to have become stalled after Ramana's question here:
> 
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00291.html
> 
> This was discussed with ARM personnel (i.e. Lee Smith) at the time the
> patch was created. AFAICT, our reasoning went like this (Paul, please
> correct me if I'm just making stuff up): we wish to use aliases for the
> old and new names for the helpers. The existing
> __gnu_{f2h,h2f}_{ieee,alternative} helpers take unsigned short
> arguments, and therefore assume (on function entry) that the values
> contained in those arguments are zero-extended (although, due to the
> vagaries of libcall handling in GCC, that may not actually be true --
> at the time, this was a latent bug, and may in fact still be). The
> __aeabi_* helpers are defined to pass/return unadorned "short" values,
> intending to convey that only the low-order 16 bits of half-float
> values contain meaningful data.

Yes, that matches my understanding.

> But, that means EABI-conformant callers are also perfectly entitled to
> sign-extend half-float values before calling our helper functions
> (although GCC itself won't do that). Using "unsigned int" and taking
> care to only examine the low-order bits of the value in the helper
> function itself serves to fix the latent bug, is compatible with
> existing code, allows us to be conformant with the eabi, and allows use
> of aliases to make the __gnu and __aeabi functions the same.

As long as LTO never sees this mismatch we should be fine :-)  AFAIK we don't 
curently have any way of expressing the actual ABI.
 
> The patch no longer applied as-is, so I've updated it (attached,
> re-tested). Note that there are no longer any target-independent changes
> (though I'm not certain that the symbol versions are still correct).
> 
> OK to apply?

I think this deserves a comment in the source.  Otherwise it's liable to get 
"fixed" in the future :-) Something allong the lines of 
"While the EABI describes the arguments to the half-float helper routines as 
'short', it does not require that they be extended to full register width.  
The normal ABI requres that the caller sign/zero extend short values to 32 
bit.  We use unsigned int arguments to prevent the gcc making assumptions 
about the high half of the register."

Paul

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

* Re: RTABI half-precision conversion functions (ping)
  2012-07-19 12:55 ` Paul Brook
@ 2012-07-19 13:48   ` Julian Brown
  2014-05-29 10:17     ` Julian Brown
  0 siblings, 1 reply; 5+ messages in thread
From: Julian Brown @ 2012-07-19 13:48 UTC (permalink / raw)
  To: Paul Brook; +Cc: gcc-patches, ams, ramana.radhakrishnan, rearnsha

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

On Thu, 19 Jul 2012 13:54:57 +0100
Paul Brook <paul@codesourcery.com> wrote:

> > But, that means EABI-conformant callers are also perfectly entitled
> > to sign-extend half-float values before calling our helper functions
> > (although GCC itself won't do that). Using "unsigned int" and taking
> > care to only examine the low-order bits of the value in the helper
> > function itself serves to fix the latent bug, is compatible with
> > existing code, allows us to be conformant with the eabi, and allows
> > use of aliases to make the __gnu and __aeabi functions the same.
> 
> As long as LTO never sees this mismatch we should be fine :-)  AFAIK
> we don't curently have any way of expressing the actual ABI.

Let's not worry about that for now :-).

> > The patch no longer applied as-is, so I've updated it (attached,
> > re-tested). Note that there are no longer any target-independent
> > changes (though I'm not certain that the symbol versions are still
> > correct).
> > 
> > OK to apply?
> 
> I think this deserves a comment in the source.  Otherwise it's liable
> to get "fixed" in the future :-) Something allong the lines of 
> "While the EABI describes the arguments to the half-float helper
> routines as 'short', it does not require that they be extended to
> full register width. The normal ABI requres that the caller sign/zero
> extend short values to 32 bit.  We use unsigned int arguments to
> prevent the gcc making assumptions about the high half of the
> register."

Here's a version with an explanatory comment. I also fixed a couple of
minor formatting nits I noticed (they don't upset the diff too much, I
don't think).

OK?

Julian

[-- Attachment #2: eabi-half-prec-fn-names-2.diff --]
[-- Type: text/x-patch, Size: 7916 bytes --]

commit f858cdd91188784794418b3456d06172df654dc3
Author: Julian Brown <jbrown@mentor.com>
Date:   Wed Jul 18 08:43:12 2012 -0700

    EABI half-precision helper function names.

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index ff46dd9..c22c2b7 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -1238,12 +1238,12 @@ arm_init_libfuncs (void)
       /* Conversions.  */
       set_conv_libfunc (trunc_optab, HFmode, SFmode,
 			(arm_fp16_format == ARM_FP16_FORMAT_IEEE
-			 ? "__gnu_f2h_ieee"
-			 : "__gnu_f2h_alternative"));
+			 ? "__aeabi_f2h"
+			 : "__aeabi_f2h_alt"));
       set_conv_libfunc (sext_optab, SFmode, HFmode,
 			(arm_fp16_format == ARM_FP16_FORMAT_IEEE
-			 ? "__gnu_h2f_ieee"
-			 : "__gnu_h2f_alternative"));
+			 ? "__aeabi_h2f"
+			 : "__aeabi_h2f_alt"));
 
       /* Arithmetic.  */
       set_optab_libfunc (add_optab, HFmode, NULL);
diff --git a/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-5.C b/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-5.C
index 92bc8a9..738d26d 100644
--- a/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-5.C
+++ b/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-5.C
@@ -13,3 +13,5 @@
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h\[a-z\]*_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h2f_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_f2h_ieee" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_h2f" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_f2h" } } */
diff --git a/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-6.C b/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-6.C
index ae40b1e..a0e09c8 100644
--- a/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-6.C
+++ b/gcc/testsuite/g++.dg/ext/arm-fp16/arm-fp16-ops-6.C
@@ -13,3 +13,5 @@
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h\[a-z\]*_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h2f_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_f2h_ieee" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_h2f" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_f2h" } } */
diff --git a/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-5.c b/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-5.c
index 92bc8a9..738d26d 100644
--- a/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-5.c
+++ b/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-5.c
@@ -13,3 +13,5 @@
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h\[a-z\]*_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h2f_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_f2h_ieee" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_h2f" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_f2h" } } */
diff --git a/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-6.c b/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-6.c
index ae40b1e..a0e09c8 100644
--- a/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-6.c
+++ b/gcc/testsuite/gcc.dg/torture/arm-fp16-ops-6.c
@@ -13,3 +13,5 @@
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h\[a-z\]*_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_h2f_ieee" } } */
 /* { dg-final { scan-assembler-not "\tbl\t__gnu_f2h_ieee" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_h2f" } } */
+/* { dg-final { scan-assembler-not "\tbl\t__aeabi_f2h" } } */
diff --git a/libgcc/config/arm/fp16.c b/libgcc/config/arm/fp16.c
index 936caeb..0fb2fe4 100644
--- a/libgcc/config/arm/fp16.c
+++ b/libgcc/config/arm/fp16.c
@@ -22,10 +22,19 @@
    see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
    <http://www.gnu.org/licenses/>.  */
 
-static inline unsigned short
-__gnu_f2h_internal(unsigned int a, int ieee)
+/* Note: While the EABI (Run-time ABI for the ARM (R) Architecture, IHI 0043C)
+   describes the helper routine arguments and return types representing
+   half-float values using the type 'short', it does not require that they be
+   extended to full register width.
+   The ABI normally requires that the caller sign or zero extends short (or
+   unsigned short) values to 32 bits.  We use unsigned int arguments and return
+   types to prevent GCC making assumptions about the high halves of the
+   registers in question.  */
+
+static inline unsigned int
+__gnu_f2h_internal (unsigned int a, int ieee)
 {
-  unsigned short sign = (a >> 16) & 0x8000;
+  unsigned int sign = (a >> 16) & 0x8000;
   int aexp = (a >> 23) & 0xff;
   unsigned int mantissa = a & 0x007fffff;
   unsigned int mask;
@@ -95,10 +104,10 @@ __gnu_f2h_internal(unsigned int a, int ieee)
   return sign | (((aexp + 14) << 10) + (mantissa >> 13));
 }
 
-unsigned int
-__gnu_h2f_internal(unsigned short a, int ieee)
+static inline unsigned int
+__gnu_h2f_internal (unsigned int a, int ieee)
 {
-  unsigned int sign = (unsigned int)(a & 0x8000) << 16;
+  unsigned int sign = (a & 0x00008000) << 16;
   int aexp = (a >> 10) & 0x1f;
   unsigned int mantissa = a & 0x3ff;
 
@@ -120,26 +129,33 @@ __gnu_h2f_internal(unsigned short a, int ieee)
   return sign | (((aexp + 0x70) << 23) + (mantissa << 13));
 }
 
-unsigned short
-__gnu_f2h_ieee(unsigned int a)
+#define ALIAS(src, dst) \
+  typeof (src) dst __attribute__ ((alias (#src)));
+
+unsigned int
+__gnu_f2h_ieee (unsigned int a)
 {
-  return __gnu_f2h_internal(a, 1);
+  return __gnu_f2h_internal (a, 1);
 }
+ALIAS (__gnu_f2h_ieee, __aeabi_f2h)
 
 unsigned int
-__gnu_h2f_ieee(unsigned short a)
+__gnu_h2f_ieee (unsigned int a)
 {
-  return __gnu_h2f_internal(a, 1);
+  return __gnu_h2f_internal (a, 1);
 }
+ALIAS (__gnu_h2f_ieee, __aeabi_h2f)
 
-unsigned short
-__gnu_f2h_alternative(unsigned int x)
+unsigned int
+__gnu_f2h_alternative (unsigned int x)
 {
-  return __gnu_f2h_internal(x, 0);
+  return __gnu_f2h_internal (x, 0);
 }
+ALIAS (__gnu_f2h_alternative, __aeabi_f2h_alt)
 
 unsigned int
-__gnu_h2f_alternative(unsigned short a)
+__gnu_h2f_alternative (unsigned int a)
 {
-  return __gnu_h2f_internal(a, 0);
+  return __gnu_h2f_internal (a, 0);
 }
+ALIAS (__gnu_h2f_alternative, __aeabi_h2f_alt)
diff --git a/libgcc/config/arm/libgcc-bpabi.ver b/libgcc/config/arm/libgcc-bpabi.ver
index 3ba8364..5bb5f04 100644
--- a/libgcc/config/arm/libgcc-bpabi.ver
+++ b/libgcc/config/arm/libgcc-bpabi.ver
@@ -106,3 +106,10 @@ GCC_3.5 {
 GCC_4.3.0 {
   _Unwind_Backtrace
 }
+
+GCC_4.7.0 {
+  __aeabi_f2h
+  __aeabi_f2h_alt
+  __aeabi_h2f
+  __aeabi_h2f_alt
+}
diff --git a/libgcc/config/arm/sfp-machine.h b/libgcc/config/arm/sfp-machine.h
index a89d05a..f2d7a37 100644
--- a/libgcc/config/arm/sfp-machine.h
+++ b/libgcc/config/arm/sfp-machine.h
@@ -99,7 +99,7 @@ typedef int __gcc_CMPtype __attribute__ ((mode (__libgcc_cmp_return__)));
 #define __fixdfdi	__aeabi_d2lz
 #define __fixunsdfdi	__aeabi_d2ulz
 #define __floatdidf	__aeabi_l2d
-#define __extendhfsf2	__gnu_h2f_ieee
-#define __truncsfhf2	__gnu_f2h_ieee
+#define __extendhfsf2	__aeabi_h2f
+#define __truncsfhf2	__aeabi_f2h
 
 #endif /* __ARM_EABI__ */
diff --git a/libgcc/config/arm/t-bpabi b/libgcc/config/arm/t-bpabi
index e79cbd7..adf977a 100644
--- a/libgcc/config/arm/t-bpabi
+++ b/libgcc/config/arm/t-bpabi
@@ -3,9 +3,8 @@ LIB1ASMFUNCS += _aeabi_lcmp _aeabi_ulcmp _aeabi_ldivmod _aeabi_uldivmod
 
 # Add the BPABI C functions.
 LIB2ADD += $(srcdir)/config/arm/bpabi.c \
-	   $(srcdir)/config/arm/unaligned-funcs.c
-
-LIB2ADD_ST += $(srcdir)/config/arm/fp16.c
+	   $(srcdir)/config/arm/unaligned-funcs.c \
+	   $(srcdir)/config/arm/fp16.c
 
 LIB2ADDEH = $(srcdir)/config/arm/unwind-arm.c \
   $(srcdir)/config/arm/libunwind.S \
diff --git a/libgcc/config/arm/t-symbian b/libgcc/config/arm/t-symbian
index d573157..248bf78 100644
--- a/libgcc/config/arm/t-symbian
+++ b/libgcc/config/arm/t-symbian
@@ -13,7 +13,7 @@ LIB1ASMFUNCS += \
 	_fixsfsi _fixunssfsi
 
 # Include half-float helpers.
-LIB2ADD_ST += $(srcdir)/config/arm/fp16.c
+LIB2ADD += $(srcdir)/config/arm/fp16.c
 
 # Include the gcc personality routine
 LIB2ADDEH = $(srcdir)/unwind-c.c $(srcdir)/config/arm/pr-support.c

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

* Re: RTABI half-precision conversion functions (ping)
  2012-07-19 13:48   ` Julian Brown
@ 2014-05-29 10:17     ` Julian Brown
  2014-06-24 11:24       ` Julian Brown
  0 siblings, 1 reply; 5+ messages in thread
From: Julian Brown @ 2014-05-29 10:17 UTC (permalink / raw)
  Cc: Paul Brook, gcc-patches, ams, ramana.radhakrishnan, rearnsha

On Thu, 19 Jul 2012 14:47:54 +0100
Julian Brown <julian@codesourcery.com> wrote:

> On Thu, 19 Jul 2012 13:54:57 +0100
> Paul Brook <paul@codesourcery.com> wrote:
> 
> > > But, that means EABI-conformant callers are also perfectly
> > > entitled to sign-extend half-float values before calling our
> > > helper functions (although GCC itself won't do that). Using
> > > "unsigned int" and taking care to only examine the low-order bits
> > > of the value in the helper function itself serves to fix the
> > > latent bug, is compatible with existing code, allows us to be
> > > conformant with the eabi, and allows use of aliases to make the
> > > __gnu and __aeabi functions the same.
> > 
> > As long as LTO never sees this mismatch we should be fine :-)  AFAIK
> > we don't curently have any way of expressing the actual ABI.
> 
> Let's not worry about that for now :-).
> 
> > > The patch no longer applied as-is, so I've updated it (attached,
> > > re-tested). Note that there are no longer any target-independent
> > > changes (though I'm not certain that the symbol versions are still
> > > correct).
> > > 
> > > OK to apply?
> > 
> > I think this deserves a comment in the source.  Otherwise it's
> > liable to get "fixed" in the future :-) Something allong the lines
> > of "While the EABI describes the arguments to the half-float helper
> > routines as 'short', it does not require that they be extended to
> > full register width. The normal ABI requres that the caller
> > sign/zero extend short values to 32 bit.  We use unsigned int
> > arguments to prevent the gcc making assumptions about the high half
> > of the register."
> 
> Here's a version with an explanatory comment. I also fixed a couple of
> minor formatting nits I noticed (they don't upset the diff too much, I
> don't think).

It looks like this one got forgotten about. Ping?

Context:

https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00902.html
https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00912.html

This is an EABI-conformance fix.

Thanks,

Julian

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

* Re: RTABI half-precision conversion functions (ping)
  2014-05-29 10:17     ` Julian Brown
@ 2014-06-24 11:24       ` Julian Brown
  0 siblings, 0 replies; 5+ messages in thread
From: Julian Brown @ 2014-06-24 11:24 UTC (permalink / raw)
  Cc: gcc-patches, ams, Ramana Radhakrishnan, rearnsha

On Thu, 29 May 2014 11:16:52 +0100
Julian Brown <julian@codesourcery.com> wrote:

> On Thu, 19 Jul 2012 14:47:54 +0100
> Julian Brown <julian@codesourcery.com> wrote:
> 
> > On Thu, 19 Jul 2012 13:54:57 +0100
> > Paul Brook <paul@codesourcery.com> wrote:
> > 
> > > > But, that means EABI-conformant callers are also perfectly
> > > > entitled to sign-extend half-float values before calling our
> > > > helper functions (although GCC itself won't do that). Using
> > > > "unsigned int" and taking care to only examine the low-order
> > > > bits of the value in the helper function itself serves to fix
> > > > the latent bug, is compatible with existing code, allows us to
> > > > be conformant with the eabi, and allows use of aliases to make
> > > > the __gnu and __aeabi functions the same.
> > > 
> > > As long as LTO never sees this mismatch we should be fine :-)
> > > AFAIK we don't curently have any way of expressing the actual ABI.
> > 
> > Let's not worry about that for now :-).
> > 
> > > > The patch no longer applied as-is, so I've updated it (attached,
> > > > re-tested). Note that there are no longer any target-independent
> > > > changes (though I'm not certain that the symbol versions are
> > > > still correct).
> > > > 
> > > > OK to apply?
> > > 
> > > I think this deserves a comment in the source.  Otherwise it's
> > > liable to get "fixed" in the future :-) Something allong the lines
> > > of "While the EABI describes the arguments to the half-float
> > > helper routines as 'short', it does not require that they be
> > > extended to full register width. The normal ABI requres that the
> > > caller sign/zero extend short values to 32 bit.  We use unsigned
> > > int arguments to prevent the gcc making assumptions about the
> > > high half of the register."
> > 
> > Here's a version with an explanatory comment. I also fixed a couple
> > of minor formatting nits I noticed (they don't upset the diff too
> > much, I don't think).
> 
> It looks like this one got forgotten about. Ping?
> 
> Context:
> 
> https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00902.html
> https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00912.html
> 
> This is an EABI-conformance fix.

Ping?

Julian

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

end of thread, other threads:[~2014-06-24 11:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-19 12:28 RTABI half-precision conversion functions (ping) Julian Brown
2012-07-19 12:55 ` Paul Brook
2012-07-19 13:48   ` Julian Brown
2014-05-29 10:17     ` Julian Brown
2014-06-24 11:24       ` Julian Brown

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