From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4928 invoked by alias); 23 Jan 2013 17:22:42 -0000 Received: (qmail 4918 invoked by uid 22791); 23 Jan 2013 17:22:40 -0000 X-SWARE-Spam-Status: No, hits=-4.7 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_GJ 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; Wed, 23 Jan 2013 17:22:34 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1Ty41x-0000g2-NR from joseph_myers@mentor.com for libc-ports@sourceware.org; Wed, 23 Jan 2013 09:22:33 -0800 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 23 Jan 2013 09:22:33 -0800 Received: from digraph.polyomino.org.uk (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; Wed, 23 Jan 2013 17:22:24 +0000 Received: from jsm28 (helo=localhost) by digraph.polyomino.org.uk with local-esmtp (Exim 4.76) (envelope-from ) id 1Ty41n-0002VM-Ju; Wed, 23 Jan 2013 17:22:23 +0000 Date: Wed, 23 Jan 2013 17:22:00 -0000 From: "Joseph S. Myers" To: "Maciej W. Rozycki" CC: , Chung-Lin Tang Subject: Re: [PATCH 2/2] MIPS16: MIPS16 support proper In-Reply-To: Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Mailing-List: contact libc-ports-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: libc-ports-owner@sourceware.org X-SW-Source: 2013-01/txt/msg00045.txt.bz2 On Wed, 23 Jan 2013, Maciej W. Rozycki wrote: > Index: ports/sysdeps/mips/__longjmp.c > =================================================================== > --- ports/sysdeps/mips/__longjmp.c 2013-01-17 00:55:17.000000000 +0000 > +++ ports/sysdeps/mips/__longjmp.c 2013-01-20 19:40:57.097753716 +0000 > @@ -23,8 +23,8 @@ > #error This file uses GNU C extensions; you must compile with GCC. > #endif > > -void > -__longjmp (env_arg, val_arg) > +static void __attribute__ ((nomips16)) > +____longjmp (env_arg, val_arg) > __jmp_buf env_arg; > int val_arg; > { > @@ -86,3 +86,5 @@ __longjmp (env_arg, val_arg) > /* Avoid `volatile function does return' warnings. */ > for (;;); > } > + > +strong_alias (____longjmp, __longjmp); Why is the renaming / alias needed? > Index: ports/sysdeps/mips/abort-instr.h > =================================================================== > --- ports/sysdeps/mips/abort-instr.h 2013-01-17 00:55:17.000000000 +0000 > +++ ports/sysdeps/mips/abort-instr.h 2013-01-20 19:40:57.097753716 +0000 > @@ -1,2 +1,6 @@ > /* An instruction which should crash any program is a breakpoint. */ > +#ifdef __mips16 > +#define ABORT_INSTRUCTION asm ("break 63") > +#else > #define ABORT_INSTRUCTION asm ("break 255") > +#endif Indentation inside #if, "# define". > +/* MIPS16 uses GCC __sync_* builtins to implement the required atomic > + operations, to abstract out the unsupported assembly instructions. > + ??? Maybe eventually use them for 32-bit MIPS too? */ > +#ifdef __mips16 > +# if __GNUC_PREREQ (4, 1) glibc requires at least 4.3 to build, so no such condition is needed in an internal header. However, __sync_* are obsolete; the __atomic_* built-in functions (supported in 4.7 and later, well-optimized for MIPS in 4.8 and later) are preferred because they allow more precise specification of the barrier semantics in particular cases. So if building with 4.8 or later, you should use the existing definitions in terms of __atomic_* that are already used in those circumstances for MIPS - and probably use them for the (4.7 and MIPS16) combination, since they won't be any worse than __sync_*. Fallbacks using __sync_* should only be for the (older GCC, MIPS16) combination. So the logic in the file should be: #if (4.8 or later, or 4.7 and MIPS16) existing implementation using __atomic_* #elif (MIPS16) implementation using __sync_*, for older GCC and MIPS16 #else existing implementation using inline asm #endif > +#else > + > +/* MIPS16 version. We currently only support O32 under MIPS16; the proper > + assembly preprocessor abstractions will need to be added if other ABIs > + are to be supported. */ > + > +#define RTLD_START asm (\ Indentation, "# define". Many further cases in this patch as well, not individually pointed out. > /* Macros for accessing the hardware control word. */ > +extern fpu_control_t __fpu_getcw (void) __THROW; > +extern void __fpu_setcw (fpu_control_t) __THROW; > +#ifdef __mips16 > +#define _FPU_GETCW(cw) do { (cw) = __fpu_getcw (); } while (0) > +#define _FPU_SETCW(cw) __fpu_setcw (cw) > +#else > #define _FPU_GETCW(cw) __asm__ volatile ("cfc1 %0,$31" : "=r" (cw)) > #define _FPU_SETCW(cw) __asm__ volatile ("ctc1 %0,$31" : : "r" (cw)) > +#endif Names __mips_fpu_getcw and __mips_fpu_setcw might reduce any risk of conflicts with any future architecture-independent internal function? > + _FPU_GETCW(cw); > + _FPU_SETCW(cw); Missing space before '('. > Index: ports/sysdeps/mips/mips32/mips16/fpu/Versions > =================================================================== > --- /dev/null 1970-01-01 00:00:00.000000000 +0000 > +++ ports/sysdeps/mips/mips32/mips16/fpu/Versions 2013-01-20 22:11:01.057025227 +0000 > @@ -0,0 +1,5 @@ > +libc { > + GLIBC_2.18 { > + __fpu_getcw; __fpu_getcw; > + } > +} The public ABI exported by glibc should not depend on whether glibc itself was built as MIPS16; remember that these functions will be needed by MIPS16 code linked with a MIPS32 glibc. So this Versions file should not be in a mips16 sysdeps directory. > Index: ports/sysdeps/mips/sys/tas.h > =================================================================== > --- ports/sysdeps/mips/sys/tas.h 2013-01-17 00:55:17.000000000 +0000 > +++ ports/sysdeps/mips/sys/tas.h 2013-01-20 19:40:57.376549265 +0000 > @@ -24,7 +24,7 @@ > > __BEGIN_DECLS > > -extern int _test_and_set (int *__p, int __v) __THROW; > +extern int __attribute__((nomips16)) test_and_set (int *__p, int __v) __THROW; > > #ifdef __USE_EXTERN_INLINES > > @@ -32,7 +32,7 @@ extern int _test_and_set (int *__p, int > # define _EXTERN_INLINE __extern_inline > # endif > > -_EXTERN_INLINE int > +_EXTERN_INLINE int __attribute__((nomips16)) > __NTH (_test_and_set (int *__p, int __v)) > { > int __r, __t; This is an installed header, so you need to use __nomips16__ rather than just plain nomips16. > Index: ports/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall.h > =================================================================== > --- /dev/null 1970-01-01 00:00:00.000000000 +0000 > +++ ports/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall.h 2013-01-20 19:40:57.546859991 +0000 > @@ -0,0 +1,91 @@ > +/* MIPS16 syscall wrappers. > + Copyright (C) 2013 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + Contributed by Maciej W. Rozycki . We don't put "Contributed by" lines in new files in glibc. > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, write to the Free > + Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA > + 02111-1307 USA. */ All files in glibc should now use a URL instead of an FSF postal address. These points apply similarly to other new files. Please send a revised patch for review. -- Joseph S. Myers joseph@codesourcery.com