Message ID | 7a7fe361-d03c-8373-51bb-52ee39b159e2@linaro.org |
---|---|
State | Dropped |
Headers |
Received: (qmail 98790 invoked by alias); 24 Aug 2017 20:08:06 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 98776 invoked by uid 89); 24 Aug 2017 20:08:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS, UNSUBSCRIBE_BODY autolearn=ham version=3.3.2 spammy= X-HELO: mail-qk0-f180.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=liabl4ynQI4v7AW7vsaaB6iKCBs4FopYmyQcvvX8Gx8=; b=qmZoW9lKN78d0y2bkkAIWJyAKrQO9yQ/d5ljZl6sdXnsWUIwN6iHDphnKDSRm9UoID jazssRttyAXJd2B2Iho6gkDUIYKrZUt/Js5Rs89X/UM4eq8FGLzn/HVMALUZ+aW1VAHi K7DAJey5p7MxnzGyWbfIPwqIfQ7fzn8Jl/tKcKbgkj7xLt+QwEYrvAd0FozOnhUtt01i 4vT4StQOtAo56OFhaQZUvCcJBLxb64FsyRQYfLcja7DG7U2twq1PK6go/10YtzTXvk2t HK1L30LobuuBzDQW79wnY3u8no0/hNXlmLU+hj4kSX7lLaDb6qWUEqLL1IuHk2sBu0jM 8w4A== X-Gm-Message-State: AHYfb5iszTzIIs0bF14258pVhjnBx+nHbsaa3LdNamMXU+B7EUHLHYj4 J9hUJg974uelDUIesyIrWA== X-Received: by 10.233.223.68 with SMTP id t65mr11246828qkf.122.1503605276774; Thu, 24 Aug 2017 13:07:56 -0700 (PDT) Subject: Re: [PATCH v5] [BZ #21956] MIPS/o32: Fix internal_syscall5/6/7 To: "Maciej W. Rozycki" <macro@imgtec.com> Cc: Aurelien Jarno <aurelien@aurel32.net>, Joseph Myers <joseph@codesourcery.com>, libc-alpha@sourceware.org References: <alpine.DEB.2.00.1708171313580.17596@tp.orcam.me.uk> <bfef7321-0a07-13bf-a153-431c244bab0a@linaro.org> <alpine.DEB.2.00.1708172114340.17596@tp.orcam.me.uk> <39ca6099-2a86-f5f4-30fe-e86bb4cd736b@linaro.org> <20170817212022.nz6wv6d55xlvubez@aurel32.net> <alpine.DEB.2.00.1708172250280.17596@tp.orcam.me.uk> <20170818071616.vk4i25nd2fkkk5ij@aurel32.net> <alpine.DEB.2.00.1708181015550.17596@tp.orcam.me.uk> <20170818174451.hhdfihr266nvpwqs@aurel32.net> <alpine.DEB.2.00.1708182212340.17596@tp.orcam.me.uk> <20170819124445.bwtvapy6w3dghucd@aurel32.net> <alpine.DEB.2.00.1708192131080.17596@tp.orcam.me.uk> <643d09c1-ecbc-bd0d-b323-2257968d0bf5@linaro.org> <alpine.DEB.2.00.1708232347420.17596@tp.orcam.me.uk> From: Adhemerval Zanella <adhemerval.zanella@linaro.org> Message-ID: <7a7fe361-d03c-8373-51bb-52ee39b159e2@linaro.org> Date: Thu, 24 Aug 2017 17:07:53 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <alpine.DEB.2.00.1708232347420.17596@tp.orcam.me.uk> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit |
Commit Message
Adhemerval Zanella Netto
Aug. 24, 2017, 8:07 p.m. UTC
On 24/08/2017 10:26, Maciej W. Rozycki wrote: > From: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > Fix a commit cc25c8b4c119 ("New pthread rwlock that is more scalable.") > regression and prevent uncontrolled stack space usage from happening > when a 5-, 6- or 7-argument syscall wrapper is placed in a loop. > > The cause of the problem is the use of `alloca' in regular MIPS/Linux > wrappers to force the use of the frame pointer register in any function > using one or more of these wrappers. Using the frame pointer register > is required so as not to break frame unwinding as the the stack pointer > is lowered within the inline asm used by these wrappers to make room for > the stack arguments, which 5-, 6- and 7-argument syscalls use with the > o32 ABI. > > The regular MIPS/Linux wrappers are macros however, expanded inline, and > stack allocations made with `alloca' are not discarded until the return > of the function they are made in. Consequently if called in a loop, > then virtual memory is wasted, and if the loop goes through enough > iterations, then ultimately available memory can get exhausted causing > the program to crash. > > Address the issue by replacing the inline code with standalone assembly > functions, which rely on the compiler arranging syscall arguments > according to the o32 function calling convention, which MIPS/Linux > syscalls also use, except for the syscall number passed and the error > flag returned. This way there is no need to fiddle with the stack > pointer anymore and all that has to be handled in the new standalone > functions is the special handling of the syscall number and the error > flag. > > Redirect 5-, 6- or 7-argument MIPS16/Linux syscall wrappers to these new > functions as well, so as to avoid an unnecessary double call the > existing wrappers would cause with the new arrangement. > > 2017-08-24 Adhemerval Zanella <adhemerval.zanella@linaro.org> > Aurelien Jarno <aurelien@aurel32.net> > Maciej W. Rozycki <macro@imgtec.com> > > [BZ #21956] > * sysdeps/unix/sysv/linux/mips/mips32/mips16/Makefile > [subdir = misc] (sysdep_routines): Remove `mips16-syscall5', > `mips16-syscall6' and `mips16-syscall7'. > (CFLAGS-mips16-syscall5.c, CFLAGS-mips16-syscall6.c) > (CFLAGS-mips16-syscall7.c): Remove. > * sysdeps/unix/sysv/linux/mips/mips32/mips16/Versions (libc): > Remove `__mips16_syscall5', `__mips16_syscall6' and > `__mips16_syscall7'. > * sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall0.c > (__mips16_syscall0): Rename `__mips16_syscall_return' to > `__mips_syscall_return'. > * sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall1.c > (__mips16_syscall1): Likewise. > * sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall2.c > (__mips16_syscall2): Likewise. > * sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall3.c > (__mips16_syscall3): Likewise. > * sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall4.c > (__mips16_syscall4): Likewise. > * sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall5.c: > Remove. > * sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall6.c: > Remove. > * sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall7.c: > Remove. > * sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall.h > (__mips16_syscall5): Expand to `__mips_syscall5' rather than > `__mips16_syscall5'. Remove prototype. > (__mips16_syscall6): Expand to `__mips_syscall6' rather than > `__mips16_syscall6'. Remove prototype. > (__mips16_syscall7): Expand to `__mips_syscall7' rather than > `__mips16_syscall7'. Remove prototype. > (__nomips16, __mips16_syscall_return): Move to... > * sysdeps/unix/sysv/linux/mips/mips32/sysdep.h > (__nomips16, __mips_syscall_return): ... here. > [__mips16] (INTERNAL_SYSCALL_NCS): Rename > `__mips16_syscall_return' to `__mips_syscall_return'. > [__mips16] (INTERNAL_SYSCALL_MIPS16): Pass `number' to > `internal_syscall##nr'. > [!__mips16] (INTERNAL_SYSCALL): Pass `SYS_ify (name)' to > `internal_syscall##nr'. > (FORCE_FRAME_POINTER): Remove. > (__mips_syscall5): New prototype. > (internal_syscall5): Rewrite to call `__mips_syscall5'. > (__mips_syscall6): New prototype. > (internal_syscall6): Rewrite to call `__mips_syscall6'. > (__mips_syscall7): New prototype. > (internal_syscall7): Rewrite to call `__mips_syscall7'. > * sysdeps/unix/sysv/linux/mips/mips32/mips-syscall5.S: New file. > * sysdeps/unix/sysv/linux/mips/mips32/mips-syscall6.S: New file. > * sysdeps/unix/sysv/linux/mips/mips32/mips-syscall7.S: New file. > * sysdeps/unix/sysv/linux/mips/mips32/Makefile [subdir = misc] > (sysdep_routines): Add libc-do-syscall. > * sysdeps/unix/sysv/linux/mips/mips32/Versions (libc): Add > `__mips_syscall5', `__mips_syscall6' and `__mips_syscall7'. Patch LGTM, thanks for following this up. > > --- > On Mon, 21 Aug 2017, Adhemerval Zanella wrote: > >>> 2. I exported `__mips_syscall?' wrappers from `libc.so' rather than making >>> them hidden. This is also consistent with `__mips16_syscall?' wrappers >>> and reduces code duplication of doubtful benefit -- it could be that >>> some calls, if internal, could be subject to the JALR->BAL >>> optimisation, however only those that are in range and only in regular >>> MIPS code, for a minimal execution time saving on some processors only. >>> Exporting these entries makes the maintenance effort much easier >>> though, as we don't have to track and record their use in the >>> individual subdirectories in Makefile. >> >> In this case we can still have internal hidden calls for libc with the cost >> of code duplication by using hiden alias with: >> >> libc_hidden_proto (__mips_syscall{5,6,7}, nomips16) >> >> And with the pairing >> >> libc_hidden_def (__mips_syscall{5,6,7}); >> >> On implementation. > > Thanks for the hint. Actually that does not cause code duplication. > > I actually considered making hidden aliases available for use within > libc.so as a possible future improvement, but didn't realise it would be > as simple to arrange with the symbols defined in standalone assembly > code rather than C. This results in 61 BAL instructions replacing JALR > ones in my regular MIPS libc.so build. Nice improvement. > >>> Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips-syscall5.S >>> =================================================================== >>> --- /dev/null 1970-01-01 00:00:00.000000000 +0000 >>> +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/mips-syscall5.S 2017-08-20 03:02:13.583854495 +0100 >>> @@ -0,0 +1,33 @@ >> >> One line comment to describe this file. > > I pinched the terse comment used across the existing MIPS16 wrappers > (with s/MIPS16/MIPS/ applied); I hope this is good enough as otherwise the > code should be self-explanatory. I think it is good enough. > >>> @@ -262,110 +275,65 @@ >>> _sys_result; \ >>> }) >>> >>> -/* We need to use a frame pointer for the functions in which we >>> - adjust $sp around the syscall, or debug information and unwind >>> - information will be $sp relative and thus wrong during the syscall. As >>> - of GCC 4.7, this is sufficient. */ >>> -#define FORCE_FRAME_POINTER \ >>> - void *volatile __fp_force __attribute__ ((unused)) = alloca (4) >>> +/* Out-of-line standard MIPS wrappers used for 5, 6, and 7 argument >>> + syscalls, which require stack arguments. */ >> >> I think it is worth to add a comment why we are using out-of-line wrappers >> for syscalls with 5, 6, and 7 arguments. > > Good point, let me know if what I came up with is comprehensive enough. > > This update has passed regular MIPS and MIPS16 o32 regression testing, > with no regressions. OK to apply? Ok from my side. > > NB while looking at it I've noticed we do not pass any `-O' optimisation > flag to the GCC driver while building .S files. That in turn means no GAS > branch optimisation is enabled and consequently branch delay slots are not > scheduled in `reorder' code by the assembler (the MIPS/GCC `asm' spec has > this `%{noasmopt:-O0; O0|fno-delayed-branch:-O1; O*:-O2; :-O1}'), causing > delay slots wasted with a NOP where a preceding instruction could be moved > instead and save some code space. It can be easily observed by comparing > code in the compiler-generated MIPS16 wrappers vs the new standalone > assembly regular MIPS (and microMIPS) wrappers. > > Was this a deliberate choice made sometime to have greater control over > code produced or just an accidental oversight? I am not sure if it was deliberate, but my guess it is not really an issue for most architectures since afaik any '-O' optimization flag along with .S files usually does not turn any extra flags (SUBTARGET_ASM_SPEC seems to be define only in a handful architectures on gcc). For MIPS I think we can set ASFLAGS to O1 since it should enable the required optimization, unless there is an specific gas option to enable it (which I couldn't find). Another option would be to filter out CFLAGS and extract the optimization level used for ASFLAGS, but I think for this specific issue it should extra non required complexity. Another thing I noticed is another possible optimization for size would to use ".set push", ".set noreorder", and ".set pop" instead of current code as below. Using GCC 6.2.1 and binutils 2.27 I noted building for mips16 (using a strip libc.so for both): - base: section size .text 966480 .eh_frame_hdr 8764 .eh_frame 36628 .pdr 97568 - patched: section size .text 966160 .eh_frame_hdr 8732 .eh_frame 36492 .pdr 97440 Not that much, but still a gain. Using O1 (as below) shows some more slight gains: section size .text 966096 .eh_frame_hdr 8732 .eh_frame 36492 .pdr 97440 --- > > Maciej > > --- > sysdeps/unix/sysv/linux/mips/mips32/Makefile | 4 > sysdeps/unix/sysv/linux/mips/mips32/Versions | 3 > sysdeps/unix/sysv/linux/mips/mips32/mips-syscall5.S | 35 ++ > sysdeps/unix/sysv/linux/mips/mips32/mips-syscall6.S | 35 ++ > sysdeps/unix/sysv/linux/mips/mips32/mips-syscall7.S | 35 ++ > sysdeps/unix/sysv/linux/mips/mips32/mips16/Makefile | 6 > sysdeps/unix/sysv/linux/mips/mips32/mips16/Versions | 2 > sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall.h | 44 -- > sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall0.c | 3 > sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall1.c | 3 > sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall2.c | 3 > sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall3.c | 3 > sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall4.c | 3 > sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall5.c | 33 -- > sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall6.c | 33 -- > sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall7.c | 33 -- > sysdeps/unix/sysv/linux/mips/mips32/sysdep.h | 163 ++++------- > 17 files changed, 201 insertions(+), 240 deletions(-) > > glibc-aurelien-mips-o32-syscall.diff > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/Makefile > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/Makefile 2017-08-20 21:30:35.093821957 +0100 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/Makefile 2017-08-22 20:33:16.504589387 +0100 > @@ -3,6 +3,10 @@ ifeq ($(subdir),conform) > conformtest-xfail-conds += mips-o32-linux > endif > > +ifeq ($(subdir),misc) > +sysdep_routines += mips-syscall5 mips-syscall6 mips-syscall7 > +endif > + > ifeq ($(subdir),stdlib) > tests += bug-getcontext-mips-gp > endif > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/Versions > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/Versions 2017-08-20 21:30:35.142707136 +0100 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/Versions 2017-08-22 20:33:16.571894966 +0100 > @@ -3,4 +3,7 @@ libc { > getrlimit64; > setrlimit64; > } > + GLIBC_PRIVATE { > + __mips_syscall5; __mips_syscall6; __mips_syscall7; > + } > } > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips-syscall5.S > =================================================================== > --- /dev/null 1970-01-01 00:00:00.000000000 +0000 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/mips-syscall5.S 2017-08-22 20:47:54.745857965 +0100 > @@ -0,0 +1,35 @@ > +/* MIPS syscall wrappers. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library. If not, see > + <http://www.gnu.org/licenses/>. */ > + > +#include <sysdep.h> > +#include <sys/asm.h> > + > + .text > + .set nomips16 > + > +/* long long __mips_syscall5 (long arg1, long arg2, long arg3, long arg4, > + long arg5, > + long number) */ > + > +ENTRY(__mips_syscall5) > + lw v0, 20(sp) > + syscall > + move v1, a3 > + jr ra > +END(__mips_syscall5) > +libc_hidden_def (__mips_syscall5) > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips-syscall6.S > =================================================================== > --- /dev/null 1970-01-01 00:00:00.000000000 +0000 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/mips-syscall6.S 2017-08-22 20:47:47.596264940 +0100 > @@ -0,0 +1,35 @@ > +/* MIPS syscall wrappers. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library. If not, see > + <http://www.gnu.org/licenses/>. */ > + > +#include <sysdep.h> > +#include <sys/asm.h> > + > + .text > + .set nomips16 > + > +/* long long __mips_syscall6 (long arg1, long arg2, long arg3, long arg4, > + long arg5, long arg6, > + long number) */ > + > +ENTRY(__mips_syscall6) > + lw v0, 24(sp) > + syscall > + move v1, a3 > + jr ra > +END(__mips_syscall6) > +libc_hidden_def (__mips_syscall6) > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips-syscall7.S > =================================================================== > --- /dev/null 1970-01-01 00:00:00.000000000 +0000 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/mips-syscall7.S 2017-08-22 20:47:25.781928113 +0100 > @@ -0,0 +1,35 @@ > +/* MIPS syscall wrappers. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library. If not, see > + <http://www.gnu.org/licenses/>. */ > + > +#include <sysdep.h> > +#include <sys/asm.h> > + > + .text > + .set nomips16 > + > +/* long long __mips_syscall7 (long arg1, long arg2, long arg3, long arg4, > + long arg5, long arg6, long arg7, > + long number) */ > + > +ENTRY(__mips_syscall7) > + lw v0, 28(sp) > + syscall > + move v1, a3 > + jr ra > +END(__mips_syscall7) > +libc_hidden_def (__mips_syscall7) > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/Makefile > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/mips16/Makefile 2017-08-20 21:30:35.448096086 +0100 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/Makefile 2017-08-22 20:33:16.687468562 +0100 > @@ -1,13 +1,9 @@ > ifeq ($(subdir),misc) > sysdep_routines += mips16-syscall0 mips16-syscall1 mips16-syscall2 > -sysdep_routines += mips16-syscall3 mips16-syscall4 mips16-syscall5 > -sysdep_routines += mips16-syscall6 mips16-syscall7 > +sysdep_routines += mips16-syscall3 mips16-syscall4 > CFLAGS-mips16-syscall0.c += -fexceptions > CFLAGS-mips16-syscall1.c += -fexceptions > CFLAGS-mips16-syscall2.c += -fexceptions > CFLAGS-mips16-syscall3.c += -fexceptions > CFLAGS-mips16-syscall4.c += -fexceptions > -CFLAGS-mips16-syscall5.c += -fexceptions > -CFLAGS-mips16-syscall6.c += -fexceptions > -CFLAGS-mips16-syscall7.c += -fexceptions > endif > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/Versions > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/mips16/Versions 2017-08-20 21:30:35.487709423 +0100 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/Versions 2017-08-22 20:33:16.719930829 +0100 > @@ -1,6 +1,6 @@ > libc { > GLIBC_PRIVATE { > __mips16_syscall0; __mips16_syscall1; __mips16_syscall2; __mips16_syscall3; > - __mips16_syscall4; __mips16_syscall5; __mips16_syscall6; __mips16_syscall7; > + __mips16_syscall4; > } > } > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall.h > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall.h 2017-08-20 21:30:35.540283348 +0100 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall.h 2017-08-22 20:33:16.732035716 +0100 > @@ -19,19 +19,6 @@ > #ifndef MIPS16_SYSCALL_H > #define MIPS16_SYSCALL_H 1 > > -#define __nomips16 __attribute__ ((nomips16)) > - > -union __mips16_syscall_return > - { > - long long val; > - struct > - { > - long v0; > - long v1; > - } > - reg; > - }; > - > long long __nomips16 __mips16_syscall0 (long number); > #define __mips16_syscall0(dummy, number) \ > __mips16_syscall0 ((long) (number)) > @@ -61,29 +48,22 @@ long long __nomips16 __mips16_syscall4 ( > (long) (a3), \ > (long) (number)) > > -long long __nomips16 __mips16_syscall5 (long a0, long a1, long a2, long a3, > - long a4, > - long number); > +/* The remaining ones use regular MIPS wrappers. */ > + > #define __mips16_syscall5(a0, a1, a2, a3, a4, number) \ > - __mips16_syscall5 ((long) (a0), (long) (a1), (long) (a2), \ > - (long) (a3), (long) (a4), \ > - (long) (number)) > + __mips_syscall5 ((long) (a0), (long) (a1), (long) (a2), \ > + (long) (a3), (long) (a4), \ > + (long) (number)) > > -long long __nomips16 __mips16_syscall6 (long a0, long a1, long a2, long a3, > - long a4, long a5, > - long number); > #define __mips16_syscall6(a0, a1, a2, a3, a4, a5, number) \ > - __mips16_syscall6 ((long) (a0), (long) (a1), (long) (a2), \ > - (long) (a3), (long) (a4), (long) (a5), \ > - (long) (number)) > + __mips_syscall6 ((long) (a0), (long) (a1), (long) (a2), \ > + (long) (a3), (long) (a4), (long) (a5), \ > + (long) (number)) > > -long long __nomips16 __mips16_syscall7 (long a0, long a1, long a2, long a3, > - long a4, long a5, long a6, > - long number); > #define __mips16_syscall7(a0, a1, a2, a3, a4, a5, a6, number) \ > - __mips16_syscall7 ((long) (a0), (long) (a1), (long) (a2), \ > - (long) (a3), (long) (a4), (long) (a5), \ > - (long) (a6), \ > - (long) (number)) > + __mips_syscall7 ((long) (a0), (long) (a1), (long) (a2), \ > + (long) (a3), (long) (a4), (long) (a5), \ > + (long) (a6), \ > + (long) (number)) > > #endif > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall0.c > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall0.c 2017-08-20 21:30:35.557630985 +0100 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall0.c 2017-08-22 20:33:16.741195496 +0100 > @@ -17,14 +17,13 @@ > <http://www.gnu.org/licenses/>. */ > > #include <sysdep.h> > -#include <mips16-syscall.h> > > #undef __mips16_syscall0 > > long long __nomips16 > __mips16_syscall0 (long number) > { > - union __mips16_syscall_return ret; > + union __mips_syscall_return ret; > ret.reg.v0 = INTERNAL_SYSCALL_MIPS16 (number, ret.reg.v1, 0); > return ret.val; > } > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall1.c > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall1.c 2017-08-20 21:30:35.658293922 +0100 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall1.c 2017-08-22 20:33:16.758455153 +0100 > @@ -17,7 +17,6 @@ > <http://www.gnu.org/licenses/>. */ > > #include <sysdep.h> > -#include <mips16-syscall.h> > > #undef __mips16_syscall1 > > @@ -25,7 +24,7 @@ long long __nomips16 > __mips16_syscall1 (long a0, > long number) > { > - union __mips16_syscall_return ret; > + union __mips_syscall_return ret; > ret.reg.v0 = INTERNAL_SYSCALL_MIPS16 (number, ret.reg.v1, 1, > a0); > return ret.val; > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall2.c > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall2.c 2017-08-20 21:30:35.769000365 +0100 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall2.c 2017-08-22 20:33:16.768703866 +0100 > @@ -17,7 +17,6 @@ > <http://www.gnu.org/licenses/>. */ > > #include <sysdep.h> > -#include <mips16-syscall.h> > > #undef __mips16_syscall2 > > @@ -25,7 +24,7 @@ long long __nomips16 > __mips16_syscall2 (long a0, long a1, > long number) > { > - union __mips16_syscall_return ret; > + union __mips_syscall_return ret; > ret.reg.v0 = INTERNAL_SYSCALL_MIPS16 (number, ret.reg.v1, 2, > a0, a1); > return ret.val; > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall3.c > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall3.c 2017-08-20 21:30:35.796726756 +0100 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall3.c 2017-08-22 20:33:16.779819073 +0100 > @@ -17,7 +17,6 @@ > <http://www.gnu.org/licenses/>. */ > > #include <sysdep.h> > -#include <mips16-syscall.h> > > #undef __mips16_syscall3 > > @@ -25,7 +24,7 @@ long long __nomips16 > __mips16_syscall3 (long a0, long a1, long a2, > long number) > { > - union __mips16_syscall_return ret; > + union __mips_syscall_return ret; > ret.reg.v0 = INTERNAL_SYSCALL_MIPS16 (number, ret.reg.v1, 3, > a0, a1, a2); > return ret.val; > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall4.c > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall4.c 2017-08-20 21:30:35.819263979 +0100 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall4.c 2017-08-22 20:33:16.794914009 +0100 > @@ -17,7 +17,6 @@ > <http://www.gnu.org/licenses/>. */ > > #include <sysdep.h> > -#include <mips16-syscall.h> > > #undef __mips16_syscall4 > > @@ -25,7 +24,7 @@ long long __nomips16 > __mips16_syscall4 (long a0, long a1, long a2, long a3, > long number) > { > - union __mips16_syscall_return ret; > + union __mips_syscall_return ret; > ret.reg.v0 = INTERNAL_SYSCALL_MIPS16 (number, ret.reg.v1, 4, > a0, a1, a2, a3); > return ret.val; > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall5.c > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall5.c 2017-08-20 21:30:35.822296212 +0100 > +++ /dev/null 1970-01-01 00:00:00.000000000 +0000 > @@ -1,33 +0,0 @@ > -/* MIPS16 syscall wrappers. > - Copyright (C) 2013-2017 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - <http://www.gnu.org/licenses/>. */ > - > -#include <sysdep.h> > -#include <mips16-syscall.h> > - > -#undef __mips16_syscall5 > - > -long long __nomips16 > -__mips16_syscall5 (long a0, long a1, long a2, long a3, > - long a4, > - long number) > -{ > - union __mips16_syscall_return ret; > - ret.reg.v0 = INTERNAL_SYSCALL_MIPS16 (number, ret.reg.v1, 5, > - a0, a1, a2, a3, a4); > - return ret.val; > -} > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall6.c > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall6.c 2017-08-20 21:30:35.838512882 +0100 > +++ /dev/null 1970-01-01 00:00:00.000000000 +0000 > @@ -1,33 +0,0 @@ > -/* MIPS16 syscall wrappers. > - Copyright (C) 2013-2017 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - <http://www.gnu.org/licenses/>. */ > - > -#include <sysdep.h> > -#include <mips16-syscall.h> > - > -#undef __mips16_syscall6 > - > -long long __nomips16 > -__mips16_syscall6 (long a0, long a1, long a2, long a3, > - long a4, long a5, > - long number) > -{ > - union __mips16_syscall_return ret; > - ret.reg.v0 = INTERNAL_SYSCALL_MIPS16 (number, ret.reg.v1, 6, > - a0, a1, a2, a3, a4, a5); > - return ret.val; > -} > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall7.c > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/mips16/mips16-syscall7.c 2017-08-20 21:30:35.846816836 +0100 > +++ /dev/null 1970-01-01 00:00:00.000000000 +0000 > @@ -1,33 +0,0 @@ > -/* MIPS16 syscall wrappers. > - Copyright (C) 2013-2017 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - <http://www.gnu.org/licenses/>. */ > - > -#include <sysdep.h> > -#include <mips16-syscall.h> > - > -#undef __mips16_syscall7 > - > -long long __nomips16 > -__mips16_syscall7 (long a0, long a1, long a2, long a3, > - long a4, long a5, long a6, > - long number) > -{ > - union __mips16_syscall_return ret; > - ret.reg.v0 = INTERNAL_SYSCALL_MIPS16 (number, ret.reg.v1, 7, > - a0, a1, a2, a3, a4, a5, a6); > - return ret.val; > -} > Index: glibc/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h > =================================================================== > --- glibc.orig/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h 2017-08-20 21:30:35.944796826 +0100 > +++ glibc/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h 2017-08-22 23:13:25.010701782 +0100 > @@ -98,6 +98,19 @@ > #undef INTERNAL_SYSCALL > #undef INTERNAL_SYSCALL_NCS > > +#define __nomips16 __attribute__ ((nomips16)) > + > +union __mips_syscall_return > + { > + long long val; > + struct > + { > + long v0; > + long v1; > + } > + reg; > + }; > + > #ifdef __mips16 > /* There's no MIPS16 syscall instruction, so we go through out-of-line > standard MIPS wrappers. These do use inline snippets below though, > @@ -112,7 +125,7 @@ > > # define INTERNAL_SYSCALL_NCS(number, err, nr, args...) \ > ({ \ > - union __mips16_syscall_return _sc_ret; \ > + union __mips_syscall_return _sc_ret; \ > _sc_ret.val = __mips16_syscall##nr (args, number); \ > err = _sc_ret.reg.v1; \ > _sc_ret.reg.v0; \ > @@ -121,13 +134,13 @@ > # define INTERNAL_SYSCALL_MIPS16(number, err, nr, args...) \ > internal_syscall##nr ("lw\t%0, %2\n\t", \ > "R" (number), \ > - 0, err, args) > + number, err, args) > > #else /* !__mips16 */ > # define INTERNAL_SYSCALL(name, err, nr, args...) \ > internal_syscall##nr ("li\t%0, %2\t\t\t# " #name "\n\t", \ > "IK" (SYS_ify (name)), \ > - 0, err, args) > + SYS_ify (name), err, args) > > # define INTERNAL_SYSCALL_NCS(number, err, nr, args...) \ > internal_syscall##nr (MOVE32 "\t%0, %2\n\t", \ > @@ -262,110 +275,74 @@ > _sys_result; \ > }) > > -/* We need to use a frame pointer for the functions in which we > - adjust $sp around the syscall, or debug information and unwind > - information will be $sp relative and thus wrong during the syscall. As > - of GCC 4.7, this is sufficient. */ > -#define FORCE_FRAME_POINTER \ > - void *volatile __fp_force __attribute__ ((unused)) = alloca (4) > +/* Standalone MIPS wrappers used for 5, 6, and 7 argument syscalls, > + which require stack arguments. We rely on the compiler arranging > + wrapper's arguments according to the MIPS o32 function calling > + convention, which is reused by syscalls, except for the syscall > + number passed and the error flag returned (taken care of in the > + wrapper called). This relieves us from relying on non-guaranteed > + compiler specifics required for the stack arguments to be pushed, > + which would be the case if these syscalls were inlined. */ > + > +long long __nomips16 __mips_syscall5 (long arg1, long arg2, long arg3, > + long arg4, long arg5, > + long number); > +libc_hidden_proto (__mips_syscall5, nomips16) > > #define internal_syscall5(v0_init, input, number, err, \ > arg1, arg2, arg3, arg4, arg5) \ > ({ \ > - long _sys_result; \ > - \ > - FORCE_FRAME_POINTER; \ > - { \ > - register long __s0 asm ("$16") __attribute__ ((unused)) \ > - = (number); \ > - register long __v0 asm ("$2"); \ > - register long __a0 asm ("$4") = (long) (arg1); \ > - register long __a1 asm ("$5") = (long) (arg2); \ > - register long __a2 asm ("$6") = (long) (arg3); \ > - register long __a3 asm ("$7") = (long) (arg4); \ > - __asm__ volatile ( \ > - ".set\tnoreorder\n\t" \ > - "subu\t$29, 32\n\t" \ > - "sw\t%6, 16($29)\n\t" \ > - v0_init \ > - "syscall\n\t" \ > - "addiu\t$29, 32\n\t" \ > - ".set\treorder" \ > - : "=r" (__v0), "+r" (__a3) \ > - : input, "r" (__a0), "r" (__a1), "r" (__a2), \ > - "r" ((long) (arg5)) \ > - : __SYSCALL_CLOBBERS); \ > - err = __a3; \ > - _sys_result = __v0; \ > - } \ > - _sys_result; \ > + union __mips_syscall_return _sc_ret; \ > + _sc_ret.val = __mips_syscall5 ((long) (arg1), \ > + (long) (arg2), \ > + (long) (arg3), \ > + (long) (arg4), \ > + (long) (arg5), \ > + (long) (number)); \ > + err = _sc_ret.reg.v1; \ > + _sc_ret.reg.v0; \ > }) > > +long long __nomips16 __mips_syscall6 (long arg1, long arg2, long arg3, > + long arg4, long arg5, long arg6, > + long number); > +libc_hidden_proto (__mips_syscall6, nomips16) > + > #define internal_syscall6(v0_init, input, number, err, \ > arg1, arg2, arg3, arg4, arg5, arg6) \ > ({ \ > - long _sys_result; \ > - \ > - FORCE_FRAME_POINTER; \ > - { \ > - register long __s0 asm ("$16") __attribute__ ((unused)) \ > - = (number); \ > - register long __v0 asm ("$2"); \ > - register long __a0 asm ("$4") = (long) (arg1); \ > - register long __a1 asm ("$5") = (long) (arg2); \ > - register long __a2 asm ("$6") = (long) (arg3); \ > - register long __a3 asm ("$7") = (long) (arg4); \ > - __asm__ volatile ( \ > - ".set\tnoreorder\n\t" \ > - "subu\t$29, 32\n\t" \ > - "sw\t%6, 16($29)\n\t" \ > - "sw\t%7, 20($29)\n\t" \ > - v0_init \ > - "syscall\n\t" \ > - "addiu\t$29, 32\n\t" \ > - ".set\treorder" \ > - : "=r" (__v0), "+r" (__a3) \ > - : input, "r" (__a0), "r" (__a1), "r" (__a2), \ > - "r" ((long) (arg5)), "r" ((long) (arg6)) \ > - : __SYSCALL_CLOBBERS); \ > - err = __a3; \ > - _sys_result = __v0; \ > - } \ > - _sys_result; \ > + union __mips_syscall_return _sc_ret; \ > + _sc_ret.val = __mips_syscall6 ((long) (arg1), \ > + (long) (arg2), \ > + (long) (arg3), \ > + (long) (arg4), \ > + (long) (arg5), \ > + (long) (arg6), \ > + (long) (number)); \ > + err = _sc_ret.reg.v1; \ > + _sc_ret.reg.v0; \ > }) > > +long long __nomips16 __mips_syscall7 (long arg1, long arg2, long arg3, > + long arg4, long arg5, long arg6, > + long arg7, > + long number); > +libc_hidden_proto (__mips_syscall7, nomips16) > + > #define internal_syscall7(v0_init, input, number, err, \ > arg1, arg2, arg3, arg4, arg5, arg6, arg7) \ > ({ \ > - long _sys_result; \ > - \ > - FORCE_FRAME_POINTER; \ > - { \ > - register long __s0 asm ("$16") __attribute__ ((unused)) \ > - = (number); \ > - register long __v0 asm ("$2"); \ > - register long __a0 asm ("$4") = (long) (arg1); \ > - register long __a1 asm ("$5") = (long) (arg2); \ > - register long __a2 asm ("$6") = (long) (arg3); \ > - register long __a3 asm ("$7") = (long) (arg4); \ > - __asm__ volatile ( \ > - ".set\tnoreorder\n\t" \ > - "subu\t$29, 32\n\t" \ > - "sw\t%6, 16($29)\n\t" \ > - "sw\t%7, 20($29)\n\t" \ > - "sw\t%8, 24($29)\n\t" \ > - v0_init \ > - "syscall\n\t" \ > - "addiu\t$29, 32\n\t" \ > - ".set\treorder" \ > - : "=r" (__v0), "+r" (__a3) \ > - : input, "r" (__a0), "r" (__a1), "r" (__a2), \ > - "r" ((long) (arg5)), "r" ((long) (arg6)), "r" ((long) (arg7)) \ > - : __SYSCALL_CLOBBERS); \ > - err = __a3; \ > - _sys_result = __v0; \ > - } \ > - _sys_result; \ > + union __mips_syscall_return _sc_ret; \ > + _sc_ret.val = __mips_syscall7 ((long) (arg1), \ > + (long) (arg2), \ > + (long) (arg3), \ > + (long) (arg4), \ > + (long) (arg5), \ > + (long) (arg6), \ > + (long) (arg7), \ > + (long) (number)); \ > + err = _sc_ret.reg.v1; \ > + _sc_ret.reg.v0; \ > }) > > #define __SYSCALL_CLOBBERS "$1", "$3", "$8", "$9", "$10", "$11", "$12", "$13", \ >
Comments
On Thu, 24 Aug 2017, Adhemerval Zanella wrote: > > This update has passed regular MIPS and MIPS16 o32 regression testing, > > with no regressions. OK to apply? > > Ok from my side. I have applied it to master now; thanks to everyone involved. As a fix for a functional regression (as demonstrated by `tst-rwlock15') do we want to have it in 2.25 and 2.26 as well? I'll continue the discussion about assembly optimisation separately. Maciej
On 2017-08-29 19:00, Maciej W. Rozycki wrote: > On Thu, 24 Aug 2017, Adhemerval Zanella wrote: > > > > This update has passed regular MIPS and MIPS16 o32 regression testing, > > > with no regressions. OK to apply? > > > > Ok from my side. > > I have applied it to master now; thanks to everyone involved. As a fix > for a functional regression (as demonstrated by `tst-rwlock15') do we want > to have it in 2.25 and 2.26 as well? That might be a good idea. At least Debian is now using this patch, and we regularly pull from the upstream stable branch. I can take care of that in the next days. Aurelien
Hi Adhemerval, Following up on the issues with MIPS assembly, both standalone sources and (as you have observed) inline pieces. On Thu, 24 Aug 2017, Adhemerval Zanella wrote: > > NB while looking at it I've noticed we do not pass any `-O' optimisation > > flag to the GCC driver while building .S files. That in turn means no GAS > > branch optimisation is enabled and consequently branch delay slots are not > > scheduled in `reorder' code by the assembler (the MIPS/GCC `asm' spec has > > this `%{noasmopt:-O0; O0|fno-delayed-branch:-O1; O*:-O2; :-O1}'), causing > > delay slots wasted with a NOP where a preceding instruction could be moved > > instead and save some code space. It can be easily observed by comparing > > code in the compiler-generated MIPS16 wrappers vs the new standalone > > assembly regular MIPS (and microMIPS) wrappers. > > > > Was this a deliberate choice made sometime to have greater control over > > code produced or just an accidental oversight? > > I am not sure if it was deliberate, but my guess it is not really an issue > for most architectures since afaik any '-O' optimization flag along with > .S files usually does not turn any extra flags (SUBTARGET_ASM_SPEC seems > to be define only in a handful architectures on gcc). > > For MIPS I think we can set ASFLAGS to O1 since it should enable the > required optimization, unless there is an specific gas option to enable > it (which I couldn't find). Another option would be to filter out CFLAGS > and extract the optimization level used for ASFLAGS, but I think for this > specific issue it should extra non required complexity. I wonder if just passing base CFLAGS unchanged would do? Anyone please correct me if I am wrong, but I believe that when building an assembly source the GCC driver is supposed to take all the usual options it accepts for C, and just silently discard those which have no effect at the assembly stage. This is in principle so that you can do say: $ gcc $CFLAGS -S foo.c $ gcc $CFLAGS -c foo.s and get exactly the same result (sans having the foo.s byproduct) as with: $ gcc $CFLAGS -c foo.c We already have to pass flags to GAS which affect multilib selection, such as `-EL' or `-mabi=', so why bother filtering? > Another thing I noticed is another possible optimization for size would > to use ".set push", ".set noreorder", and ".set pop" instead of current > code as below. > [...] > > diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h > index dadfa18..96867de 100644 > --- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h > +++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h > @@ -159,10 +159,11 @@ union __mips_syscall_return > register long __v0 asm ("$2"); \ > register long __a3 asm ("$7"); \ > __asm__ volatile ( \ > - ".set\tnoreorder\n\t" \ > + ".set push\n\t" \ > + ".set noreorder\n\t" \ > v0_init \ > "syscall\n\t" \ > - ".set reorder" \ > + ".set pop" \ > : "=r" (__v0), "=r" (__a3) \ > : input \ > : __SYSCALL_CLOBBERS); \ [Etc...] If this makes any difference (and your size change observations do not merely come from updated $ASFLAGS), then this is very weird and I think it needs investigating. The thing is for backwards compatibility at the place where an inline asm is expanded GCC sets the `reorder' mode (unless it's been set already), as this is what was the GCC's mode of assembly generation before support for explicit relocations was added back in early-mid 2000s. If switching back to the `noreorder' is required by GCC at the conclusion of the asm, it has to do the switch itself. So there should actually be no difference in machine code generated between current code and your patched version of sysdep.h. If there is, then something hairy is going on. I'll try experimenting with your patch and will check what's happening here. Maciej
diff --git a/sysdeps/mips/Makefile b/sysdeps/mips/Makefile index 7c1d779..1130015 100644 --- a/sysdeps/mips/Makefile +++ b/sysdeps/mips/Makefile @@ -83,3 +83,9 @@ $(objpfx)tst-mode-switch-2: $(shared-thread-library) endif endif endif + +# Enable delay branch optimization +ASFLAGS-.o += -O1 +ASFLAGS-.os += -O1 +ASFLAGS-.op += -O1 +ASFLAGS-.oS += -O2 --- diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h index dadfa18..96867de 100644 --- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h +++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h @@ -159,10 +159,11 @@ union __mips_syscall_return register long __v0 asm ("$2"); \ register long __a3 asm ("$7"); \ __asm__ volatile ( \ - ".set\tnoreorder\n\t" \ + ".set push\n\t" \ + ".set noreorder\n\t" \ v0_init \ "syscall\n\t" \ - ".set reorder" \ + ".set pop" \ : "=r" (__v0), "=r" (__a3) \ : input \ : __SYSCALL_CLOBBERS); \ @@ -183,10 +184,11 @@ union __mips_syscall_return register long __a0 asm ("$4") = (long) (arg1); \ register long __a3 asm ("$7"); \ __asm__ volatile ( \ - ".set\tnoreorder\n\t" \ + ".set push\n\t" \ + ".set noreorder\n\t" \ v0_init \ "syscall\n\t" \ - ".set reorder" \ + ".set pop" \ : "=r" (__v0), "=r" (__a3) \ : input, "r" (__a0) \ : __SYSCALL_CLOBBERS); \ @@ -208,10 +210,11 @@ union __mips_syscall_return register long __a1 asm ("$5") = (long) (arg2); \ register long __a3 asm ("$7"); \ __asm__ volatile ( \ - ".set\tnoreorder\n\t" \ + ".set push\n\t" \ + ".set noreorder\n\t" \ v0_init \ "syscall\n\t" \ - ".set\treorder" \ + ".set pop" \ : "=r" (__v0), "=r" (__a3) \ : input, "r" (__a0), "r" (__a1) \ : __SYSCALL_CLOBBERS); \ @@ -235,10 +238,11 @@ union __mips_syscall_return register long __a2 asm ("$6") = (long) (arg3); \ register long __a3 asm ("$7"); \ __asm__ volatile ( \ - ".set\tnoreorder\n\t" \ + ".set push\n\t" \ + ".set noreorder\n\t" \ v0_init \ "syscall\n\t" \ - ".set\treorder" \ + ".set pop" \ : "=r" (__v0), "=r" (__a3) \ : input, "r" (__a0), "r" (__a1), "r" (__a2) \ : __SYSCALL_CLOBBERS); \ @@ -262,10 +266,11 @@ union __mips_syscall_return register long __a2 asm ("$6") = (long) (arg3); \ register long __a3 asm ("$7") = (long) (arg4); \ __asm__ volatile ( \ - ".set\tnoreorder\n\t" \ + ".set push\n\t" \ + ".set noreorder\n\t" \ v0_init \ "syscall\n\t" \ - ".set\treorder" \ + ".set pop" \ : "=r" (__v0), "+r" (__a3) \ : input, "r" (__a0), "r" (__a1), "r" (__a2) \ : __SYSCALL_CLOBBERS); \