diff mbox

[i386] Remove EBX usage from asm code

Message ID CAFULd4bACGP0MrWjp+AaLOLUKsStpnSCeCYUMmcNTj1Yqt7YCw@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak Jan. 23, 2015, 11:56 a.m. UTC
On Tue, Jan 20, 2015 at 10:58 AM, Jakub Jelinek <jakub@redhat.com> wrote:

>> >>> The target is i386-pc-solaris2.10, which includes i386/sysv4.h.  Only
>> >>> the amd64 crtbegin.o is affected, the i386 one is fine.
>> >>
>> >> Please split sysv4-common.h out of i386/sysv4.h, similar to how
>> >> i386/gnu-user.h and gnu-user-common.h are split.
>> >
>> > This would break Solaris/SPARC (there's no sparc/sysv4-common.h), and
>> > only works on Linux/x86 by accident:
>> >
>> > * CRT_GET_RFIB_DATA is only defined in i386/gnu-user.h there, but that
>> >   file is only included for 32-bit-only configurations, not
>> >   32-bit-default ones (--enable-targets=all).  In the latter case, the
>> >   macro is not defined for the 32-bit multilib, where it should be.
>> >
>> > * CRT_GET_RFIB_DATA is only used in libgcc/crtstuff.c and
>> >   libgcc/unwind-dw2-fde-dip.c (which already has a workaround for it
>> >   being incorrectly defined for 64-bit Solaris/x86).
>> >
>> > IMO, the definition has no business living in gcc/config/i386 at all,
>> > but should move to libgcc/config instead (together with the one in
>> > frv/frv.h).  That being probably too intrusive at this stage, I think
>> > the best workaround for now is to simply wrap the definition in
>> > i386/syv4.h (which is Solaris/x86-only anyway) in __i386__.
>>
>> Ugh...
>>
>> Considering your explanation and the mess in the unwinder, IMO this
>> should be fixed in the correct way even at this stage.
>>
>> CC RMs for their opinion.
>
> Agreed.

I'm testing the attached patch that moves the definition to libgcc
*and* wraps it in __i386__ to fix multilibs. Rainer, can you please
implement the fix for the Solaris/x86, presumably in the same way?

libgcc/ChangeLog:

2015-23-01  Uros Bizjak  <ubizjak@gmail.com>

    * config/i386/gnu-user-lib.h New file.
    (CRT_GET_RFIB_DATA): Move definition from gcc/config/i386/gnu-user.h.
    Wrap definition in #ifdef __i386__.
    * libgcc/config.host (i[34567]86-*-linux*, i[34567]86-*-kfreebsd*-gnu,
    i[34567]86-*-knetbsd*-gnu, i[34567]86-*-gnu*,
    i[34567]86-*-kopensolaris*-gnu, x86_64-*-linux*,
    x86_64-*-kfreebsd*-gnu, x86_64-*-knetbsd*-gnu): Add i386/gnu-user-lib.h
    to tm_file.

gcc/ChangeLog:

2015-23-01  Uros Bizjak  <ubizjak@gmail.com>

    * config/i386/gnu-user.h (CRT_GET_RFIB_DATA): Move definition to
    libgcc/config/i386/gnu-user-lib.h.

Bootstrap and regression test on x86_64-linux-gnu {,-m32} in progress.

Uros.

Comments

Jakub Jelinek Jan. 23, 2015, 12:04 p.m. UTC | #1
On Fri, Jan 23, 2015 at 12:56:02PM +0100, Uros Bizjak wrote:
> I'm testing the attached patch that moves the definition to libgcc
> *and* wraps it in __i386__ to fix multilibs. Rainer, can you please
> implement the fix for the Solaris/x86, presumably in the same way?
> 
> libgcc/ChangeLog:
> 
> 2015-23-01  Uros Bizjak  <ubizjak@gmail.com>
> 
>     * config/i386/gnu-user-lib.h New file.

Missing semicolon.
I'd argue that there is nothing GNU related in it though, _GLOBAL_OFFSET_TABLE_
is a general ELF thing, so perhaps elf-lib.h instead?

	Jakub
Uros Bizjak Jan. 23, 2015, 12:08 p.m. UTC | #2
On Fri, Jan 23, 2015 at 1:04 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Jan 23, 2015 at 12:56:02PM +0100, Uros Bizjak wrote:
>> I'm testing the attached patch that moves the definition to libgcc
>> *and* wraps it in __i386__ to fix multilibs. Rainer, can you please
>> implement the fix for the Solaris/x86, presumably in the same way?
>>
>> libgcc/ChangeLog:
>>
>> 2015-23-01  Uros Bizjak  <ubizjak@gmail.com>
>>
>>     * config/i386/gnu-user-lib.h New file.
>
> Missing semicolon.
> I'd argue that there is nothing GNU related in it though, _GLOBAL_OFFSET_TABLE_
> is a general ELF thing, so perhaps elf-lib.h instead?

Yes, the name is better. It would also fit with Solaris.

Uros.
Uros Bizjak Jan. 23, 2015, 7:48 p.m. UTC | #3
On Fri, Jan 23, 2015 at 1:08 PM, Uros Bizjak <ubizjak@gmail.com> wrote:

>>> I'm testing the attached patch that moves the definition to libgcc
>>> *and* wraps it in __i386__ to fix multilibs. Rainer, can you please
>>> implement the fix for the Solaris/x86, presumably in the same way?
>>>
>>> libgcc/ChangeLog:
>>>
>>> 2015-23-01  Uros Bizjak  <ubizjak@gmail.com>
>>>
>>>     * config/i386/gnu-user-lib.h New file.
>>
>> Missing semicolon.
>> I'd argue that there is nothing GNU related in it though, _GLOBAL_OFFSET_TABLE_
>> is a general ELF thing, so perhaps elf-lib.h instead?
>
> Yes, the name is better. It would also fit with Solaris.

Bootstrap and regression test was OK.

Committed with following ChangeLogs:

libgcc/ChangeLog:

2015-01-23  Uros Bizjak  <ubizjak@gmail.com>

    * config/i386/elf-lib.h: New file.
    (CRT_GET_RFIB_DATA): Move definition from gcc/config/i386/gnu-user.h.
    Wrap definition in #ifdef __i386__.
    * libgcc/config.host (i[34567]86-*-linux*, i[34567]86-*-kfreebsd*-gnu)
    (i[34567]86-*-knetbsd*-gnu, i[34567]86-*-gnu*)
    (i[34567]86-*-kopensolaris*-gnu, x86_64-*-linux*)
    (x86_64-*-kfreebsd*-gnu, x86_64-*-knetbsd*-gnu): Add i386/elf-lib.h
    to tm_file.

gcc/ChangeLog:

2015-01-23  Uros Bizjak  <ubizjak@gmail.com>

    * config/i386/gnu-user.h (CRT_GET_RFIB_DATA): Move definition to
    libgcc/config/i386/elf-lib.h.

Uros.
diff mbox

Patch

Index: libgcc/config.host
===================================================================
--- libgcc/config.host	(revision 220027)
+++ libgcc/config.host	(working copy)
@@ -585,20 +585,24 @@ 
 i[34567]86-*-linux*)
 	extra_parts="$extra_parts crtprec32.o crtprec64.o crtprec80.o crtfastmath.o"
 	tmake_file="${tmake_file} i386/t-crtpc i386/t-crtfm i386/t-crtstuff t-dfprules"
+	tm_file="${tm_file} i386/gnu-user-lib.h"
 	md_unwind_header=i386/linux-unwind.h
 	;;
 i[34567]86-*-kfreebsd*-gnu | i[34567]86-*-knetbsd*-gnu | i[34567]86-*-gnu* | i[34567]86-*-kopensolaris*-gnu)
 	extra_parts="$extra_parts crtprec32.o crtprec64.o crtprec80.o crtfastmath.o"
 	tmake_file="${tmake_file} i386/t-crtpc i386/t-crtfm i386/t-crtstuff t-dfprules"
+	tm_file="${tm_file} i386/gnu-user-lib.h"
 	;;
 x86_64-*-linux*)
 	extra_parts="$extra_parts crtprec32.o crtprec64.o crtprec80.o crtfastmath.o"
 	tmake_file="${tmake_file} i386/t-crtpc i386/t-crtfm i386/t-crtstuff t-dfprules"
+	tm_file="${tm_file} i386/gnu-user-lib.h"
 	md_unwind_header=i386/linux-unwind.h
 	;;
 x86_64-*-kfreebsd*-gnu | x86_64-*-knetbsd*-gnu)
 	extra_parts="$extra_parts crtprec32.o crtprec64.o crtprec80.o crtfastmath.o"
 	tmake_file="${tmake_file} i386/t-crtpc i386/t-crtfm i386/t-crtstuff t-dfprules"
+	tm_file="${tm_file} i386/gnu-user-lib.h"
 	;;
 i[34567]86-pc-msdosdjgpp*)
 	;;
Index: libgcc/config/i386/gnu-user-lib.h
===================================================================
--- libgcc/config/i386/gnu-user-lib.h	(revision 0)
+++ libgcc/config/i386/gnu-user-lib.h	(revision 0)
@@ -0,0 +1,36 @@ 
+/* Definitions for Intel 386 systems using GNU userspace.
+   Copyright (C) 2015 Free Software Foundation, Inc.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC 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 General Public License
+for more details.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+<http://www.gnu.org/licenses/>.  */
+
+#ifdef __i386__
+/* Used by crtstuff.c to initialize the base of data-relative relocations.
+   These are GOT relative on x86, so return the pic register.  */
+#define CRT_GET_RFIB_DATA(BASE)						\
+  __asm__ ("call\t.LPR%=\n"						\
+	   ".LPR%=:\n\t"						\
+	   "pop{l}\t%0\n\t"						\
+	   /* Due to a GAS bug, this cannot use EAX.  That encodes	\
+	      smaller than the traditional EBX, which results in the	\
+	      offset being off by one.  */				\
+	   "add{l}\t{$_GLOBAL_OFFSET_TABLE_+[.-.LPR%=],%0"		\
+		   "|%0,_GLOBAL_OFFSET_TABLE_+(.-.LPR%=)}"		\
+	   : "=d"(BASE))
+#endif
Index: gcc/config/i386/gnu-user.h
===================================================================
--- gcc/config/i386/gnu-user.h	(revision 220027)
+++ gcc/config/i386/gnu-user.h	(working copy)
@@ -129,19 +129,6 @@ 
       }									\
   } while (0)
 
-/* Used by crtstuff.c to initialize the base of data-relative relocations.
-   These are GOT relative on x86, so return the pic register.  */
-#define CRT_GET_RFIB_DATA(BASE)						\
-  __asm__ ("call\t.LPR%=\n"						\
-	   ".LPR%=:\n\t"						\
-	   "pop{l}\t%0\n\t"						\
-	   /* Due to a GAS bug, this cannot use EAX.  That encodes	\
-	      smaller than the traditional EBX, which results in the	\
-	      offset being off by one.  */				\
-	   "add{l}\t{$_GLOBAL_OFFSET_TABLE_+[.-.LPR%=],%0"		\
-		   "|%0,_GLOBAL_OFFSET_TABLE_+(.-.LPR%=)}"		\
-	   : "=d"(BASE))
-
 #ifdef TARGET_LIBC_PROVIDES_SSP
 /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
 #define TARGET_THREAD_SSP_OFFSET	0x14