From patchwork Thu Jul 19 13:47:54 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julian Brown X-Patchwork-Id: 171948 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id A809A2C0268 for ; Thu, 19 Jul 2012 23:48:27 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1343310508; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Date:From:To:CC:Subject:Message-ID:In-Reply-To: References:MIME-Version:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=opgSqc+/zf+/XSN6GmmoG3uLb90=; b=W8cnJFRyIlBoJnK uov2j/TZvO33Btf/cqYkf8+P7pGJrX8+qxGAN+FGEwGb/ps+TDlXs9mgNNT2CcPi ZRehnYwdETOcVVqkTL+O8/c5spkRUujlF7Np0r3I3oLbCDGC/h2ImzXxLHe6TKB2 eHTqIADHNpbPxcUyzu5BN0zsprMs= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Date:From:To:CC:Subject:Message-ID:In-Reply-To:References:MIME-Version:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=iOKDzYT0dGyiaKIonOouTZEntAdA7hAOsTSoFxQ7y5aQDQPEMbQurlBgvDpkG6 HOu5FOnEPEANvO0cv72HjsZIZEuBp6L1V69kmfdus1Ar/Jdg6n7lvIFBRhGWYE8w OrQIg4XQ5WQMozsxOYQIPNYltdeQwqGMfRrVMsp73Yr7U=; Received: (qmail 15356 invoked by alias); 19 Jul 2012 13:48:22 -0000 Received: (qmail 15346 invoked by uid 22791); 19 Jul 2012 13:48:21 -0000 X-SWARE-Spam-Status: No, hits=-2.8 required=5.0 tests=AWL, BAYES_50, KHOP_RCVD_UNTRUST, KHOP_THREADED, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, TW_DH, TW_FH, TW_IV, TW_SF, TW_XD, TW_XS X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 19 Jul 2012 13:48:06 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1Srr5I-0005ql-NE from Julian_Brown@mentor.com ; Thu, 19 Jul 2012 06:48:04 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Thu, 19 Jul 2012 06:48:04 -0700 Received: from octopus (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Thu, 19 Jul 2012 14:48:01 +0100 Date: Thu, 19 Jul 2012 14:47:54 +0100 From: Julian Brown To: Paul Brook CC: , , , Subject: Re: RTABI half-precision conversion functions (ping) Message-ID: <20120719144754.514db0e3@octopus> In-Reply-To: <201207191354.58297.paul@codesourcery.com> References: <20120719132816.2b51c0bc@octopus> <201207191354.58297.paul@codesourcery.com> MIME-Version: 1.0 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Thu, 19 Jul 2012 13:54:57 +0100 Paul Brook 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 commit f858cdd91188784794418b3456d06172df654dc3 Author: Julian Brown 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 . */ -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