Discussion:
[Patch, MIPS] Cleanup mips header files.
Steve Ellcey
2014-09-26 22:39:16 UTC
Permalink
I would like to do some cleanup on the mips configuration code, both to
reduce the amount of duplicated code and to add better support for --with-arch,
--with-endian, and --with-abi. As the first step in this work I would like
to check in this patch that removes the linux64.h and gnu-user64.h header
files and copies the needed pieces to linux.h and gnu-user.h.

Right now these headers are used when building a mips64* target or if you
use --enable-target=all, but there is no reason they can't be used for normal
32 bit mips targets too. Then the only thing has to be done differently for
mips64* targets (or --enable-target=all) vs. mips 32 bit targets is to add
the multilib makefile fragment (t-linux64).

Most of the changes here are just moving macros from one file to another.
The only real functional changes are with GNU_USER_TARGET_LINK_SPEC and
LINUX_DRIVER_SELF_SPECS where we pass more explicit options to the linker
and now have a single consistent definition of these macros for all mips
targets instead of different ones for mips32 and mips64.

I built multiple different mips*-*-linux-gnu targets to test this change
but there are a lot of combinations and I couldn't build all of them.

OK for checkin?

Steve Ellcey
***@mips.com



2014-09-26 Steve Ellcey <***@mips.com>

* config/mips/linux64.h: Remove.
* config/mips/gnu-user64.h: Remove.
* gcc.config (mips*-*-*): Remove references to linux64.h and
gnu-user64.h
* config/mips/gnu-user.h (GNU_USER_TARGET_LINK_SPEC): Replace
with modified version from gnu-user64.h.
(LINUX_DRIVER_SELF_SPECS): Update parts from gnu-user64.h.
(LOCAL_LABEL_PREFIX): Copy from gnu-user64.h.
* config/mips/linux.h (GNU_USER_LINK_EMULATION32): Copy from
linux64.h.
(GNU_USER_LINK_EMULATION64): Ditto.
(GNU_USER_LINK_EMULATIONN32): Ditto.
(GLIBC_DYNAMIC_LINKER32): Ditto.
(GLIBC_DYNAMIC_LINKER64): Ditto.
(GLIBC_DYNAMIC_LINKERN32): Ditto.
(UCLIBC_DYNAMIC_LINKER32): Ditto.
(UCLIBC_DYNAMIC_LINKER64): Ditto.
(UCLIBC_DYNAMIC_LINKERN32): Ditto.
(BIONIC_DYNAMIC_LINKERN32): Ditto.
(GNU_USER_DYNAMIC_LINKERN32): Ditto.
(GLIBC_DYNAMIC_LINKER): Delete.
(UCLIBC_DYNAMIC_LINKER): Delete.


diff --git a/gcc/config.gcc b/gcc/config.gcc
index 0e50e9a..ce06656 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1941,7 +1941,7 @@ mips*-*-netbsd*) # NetBSD/mips, either endian.
extra_options="${extra_options} netbsd.opt netbsd-elf.opt"
;;
mips*-mti-linux*)
- tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-stdint.h ${tm_file} mips/gnu-user.h mips/gnu-user64.h mips/linux64.h mips/linux-common.h mips/mti-linux.h"
+ tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-stdint.h ${tm_file} mips/gnu-user.h mips/linux.h mips/linux-common.h mips/mti-linux.h"
extra_options="${extra_options} linux-android.opt"
tmake_file="${tmake_file} mips/t-mti-linux"
tm_defines="${tm_defines} MIPS_ISA_DEFAULT=33 MIPS_ABI_DEFAULT=ABI_32"
@@ -1949,7 +1949,7 @@ mips*-mti-linux*)
gas=yes
;;
mips64*-*-linux* | mipsisa64*-*-linux*)
- tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-stdint.h ${tm_file} mips/gnu-user.h mips/gnu-user64.h mips/linux64.h mips/linux-common.h"
+ tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-stdint.h ${tm_file} mips/gnu-user.h mips/linux.h mips/linux-common.h"
extra_options="${extra_options} linux-android.opt"
tmake_file="${tmake_file} mips/t-linux64"
tm_defines="${tm_defines} MIPS_ABI_DEFAULT=ABI_N32"
@@ -1973,7 +1973,6 @@ mips*-*-linux*) # Linux MIPS, either endian.
tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h glibc-stdint.h ${tm_file} mips/gnu-user.h mips/linux.h"
extra_options="${extra_options} linux-android.opt"
if test x$enable_targets = xall; then
- tm_file="${tm_file} mips/gnu-user64.h mips/linux64.h"
tmake_file="${tmake_file} mips/t-linux64"
fi
tm_file="${tm_file} mips/linux-common.h"
diff --git a/gcc/config/mips/gnu-user.h b/gcc/config/mips/gnu-user.h
index 638d7f0..3bb2248 100644
--- a/gcc/config/mips/gnu-user.h
+++ b/gcc/config/mips/gnu-user.h
@@ -52,16 +52,20 @@ along with GCC; see the file COPYING3. If not see
#undef MIPS_DEFAULT_GVALUE
#define MIPS_DEFAULT_GVALUE 0

-/* Borrowed from sparc/linux.h */
#undef GNU_USER_TARGET_LINK_SPEC
-#define GNU_USER_TARGET_LINK_SPEC \
- "%(endian_spec) \
- %{shared:-shared} \
+#define GNU_USER_TARGET_LINK_SPEC "\
+%{G*} %{EB} %{EL} %{!EB:%{!EL:%(endian_spec)}} %{mips*} \
+%{shared} \
%{!shared: \
%{!static: \
%{rdynamic:-export-dynamic} \
- -dynamic-linker " GNU_USER_DYNAMIC_LINKER "} \
- %{static:-static}}"
+ %{mabi=n32: -dynamic-linker " GNU_USER_DYNAMIC_LINKERN32 "} \
+ %{mabi=64: -dynamic-linker " GNU_USER_DYNAMIC_LINKER64 "} \
+ %{mabi=32: -dynamic-linker " GNU_USER_DYNAMIC_LINKER32 "}} \
+ %{static:-static}} \
+%{mabi=n32:-m" GNU_USER_LINK_EMULATIONN32 "} \
+%{mabi=64:-m" GNU_USER_LINK_EMULATION64 "} \
+%{mabi=32:-m" GNU_USER_LINK_EMULATION32 "}"
#undef LINK_SPEC
#define LINK_SPEC GNU_USER_TARGET_LINK_SPEC

@@ -122,7 +126,9 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
specs handling by removing a redundant option. */ \
"%{!mno-shared:%<mplt}", \
/* -mplt likewise has no effect for -mabi=64 without -msym32. */ \
- "%{mabi=64:%{!msym32:%<mplt}}"
+ "%{mabi=64:%{!msym32:%<mplt}}" \
+ " %{!EB:%{!EL:%(endian_spec)}}" \
+ " %{!mabi=*: -" MULTILIB_ABI_DEFAULT "}"

#undef DRIVER_SELF_SPECS
#define DRIVER_SELF_SPECS \
@@ -137,3 +143,6 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
#define ENDFILE_SPEC \
GNU_USER_TARGET_MATHFILE_SPEC " " \
GNU_USER_TARGET_ENDFILE_SPEC
+
+#undef LOCAL_LABEL_PREFIX
+#define LOCAL_LABEL_PREFIX (TARGET_OLDABI ? "$" : ".")
diff --git a/gcc/config/mips/linux.h b/gcc/config/mips/linux.h
index e539422..a117f90 100644
--- a/gcc/config/mips/linux.h
+++ b/gcc/config/mips/linux.h
@@ -17,9 +17,27 @@ You should have received a copy of the GNU General Public License
along with GCC; see the file COPYING3. If not see
<http://www.gnu.org/licenses/>. */

-#define GLIBC_DYNAMIC_LINKER \
+#define GNU_USER_LINK_EMULATION32 "elf32%{EB:b}%{EL:l}tsmip"
+#define GNU_USER_LINK_EMULATION64 "elf64%{EB:b}%{EL:l}tsmip"
+#define GNU_USER_LINK_EMULATIONN32 "elf32%{EB:b}%{EL:l}tsmipn32"
+
+#define GLIBC_DYNAMIC_LINKER32 \
"%{mnan=2008:/lib/ld-linux-mipsn8.so.1;:/lib/ld.so.1}"
+#define GLIBC_DYNAMIC_LINKER64 \
+ "%{mnan=2008:/lib64/ld-linux-mipsn8.so.1;:/lib64/ld.so.1}"
+#define GLIBC_DYNAMIC_LINKERN32 \
+ "%{mnan=2008:/lib32/ld-linux-mipsn8.so.1;:/lib32/ld.so.1}"

-#undef UCLIBC_DYNAMIC_LINKER
-#define UCLIBC_DYNAMIC_LINKER \
+#undef UCLIBC_DYNAMIC_LINKER32
+#define UCLIBC_DYNAMIC_LINKER32 \
"%{mnan=2008:/lib/ld-uClibc-mipsn8.so.0;:/lib/ld-uClibc.so.0}"
+#undef UCLIBC_DYNAMIC_LINKER64
+#define UCLIBC_DYNAMIC_LINKER64 \
+ "%{mnan=2008:/lib/ld64-uClibc-mipsn8.so.0;:/lib/ld64-uClibc.so.0}"
+#define UCLIBC_DYNAMIC_LINKERN32 \
+ "%{mnan=2008:/lib32/ld-uClibc-mipsn8.so.0;:/lib32/ld-uClibc.so.0}"
+
+#define BIONIC_DYNAMIC_LINKERN32 "/system/bin/linker32"
+#define GNU_USER_DYNAMIC_LINKERN32 \
+ CHOOSE_DYNAMIC_LINKER (GLIBC_DYNAMIC_LINKERN32, UCLIBC_DYNAMIC_LINKERN32, \
+ BIONIC_DYNAMIC_LINKERN32)
Matthew Fortune
2014-10-06 21:25:47 UTC
Permalink
Hi Steve,

You're the lucky recipient of my first review so apologies for being
slow and cautious...

I tried to find a reason why the files were originally separated like this
and I can't see anything obvious. I assume you also found no reason.
Presumably the separation was just to avoid disturbing the 32-bit configs
but I think it is a sensible move to merge them.
Post by Steve Ellcey
--- a/gcc/config/mips/gnu-user.h
+++ b/gcc/config/mips/gnu-user.h
@@ -52,16 +52,20 @@ along with GCC; see the file COPYING3. If not see
#undef MIPS_DEFAULT_GVALUE
#define MIPS_DEFAULT_GVALUE 0
-/* Borrowed from sparc/linux.h */
#undef GNU_USER_TARGET_LINK_SPEC
-#define GNU_USER_TARGET_LINK_SPEC \
- "%(endian_spec) \
- %{shared:-shared} \
+#define GNU_USER_TARGET_LINK_SPEC "\
+%{G*} %{EB} %{EL} %{!EB:%{!EL:%(endian_spec)}} %{mips*} \
+%{shared} \
Indent two lines above. The '%{!EB:%{!EL:%(endian_spec)}}' is
redundant as it is covered by the DRIVER_SELF_SPEC which you have added
below. The linker won't accept -mips16 so you should use
MIPS_ISA_LEVEL_OPTION_SPEC to avoid passing mips16. Although we still
end up with an explicit list at least there is just one copy to maintain.
Post by Steve Ellcey
%{!shared: \
%{!static: \
%{rdynamic:-export-dynamic} \
- -dynamic-linker " GNU_USER_DYNAMIC_LINKER "} \
- %{static:-static}}"
+ %{mabi=n32: -dynamic-linker " GNU_USER_DYNAMIC_LINKERN32 "} \
+ %{mabi=64: -dynamic-linker " GNU_USER_DYNAMIC_LINKER64 "} \
+ %{mabi=32: -dynamic-linker " GNU_USER_DYNAMIC_LINKER32 "}} \
+ %{static:-static}} \
Very much a nit but if we are going to have %{shared} above then we should
have just %{static} here.
Post by Steve Ellcey
+%{mabi=n32:-m" GNU_USER_LINK_EMULATIONN32 "} \
+%{mabi=64:-m" GNU_USER_LINK_EMULATION64 "} \
+%{mabi=32:-m" GNU_USER_LINK_EMULATION32 "}"
#undef LINK_SPEC
#define LINK_SPEC GNU_USER_TARGET_LINK_SPEC
@@ -122,7 +126,9 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
specs handling by removing a redundant option. */ \
"%{!mno-shared:%<mplt}", \
/* -mplt likewise has no effect for -mabi=64 without -msym32. */ \
- "%{mabi=64:%{!msym32:%<mplt}}"
+ "%{mabi=64:%{!msym32:%<mplt}}" \
+ " %{!EB:%{!EL:%(endian_spec)}}" \
+ " %{!mabi=*: -" MULTILIB_ABI_DEFAULT "}"
Another nit, since this code is being changed can you update this from using
Post by Steve Ellcey
+ "%{mabi=64:%{!msym32:%<mplt}}", \
+ "%{!EB:%{!EL:%(endian_spec)}}", \
+ "%{!mabi=*: -" MULTILIB_ABI_DEFAULT "}"
With those changes can you double check that a default big and default little
endian build pass -EB/-EL respectively by default and are changed when using
-EL/-EB explicitly? There should only be one -E* option passed to the linker
theoretically, unless multiple explicit -E* options are given on the command
line.

Otherwise OK (assuming the link specs behave as described above).

Thanks,
Matthew
Steve Ellcey
2014-10-08 16:22:08 UTC
Permalink
Post by Matthew Fortune
Hi Steve,
You're the lucky recipient of my first review so apologies for being
slow and cautious...
I tried to find a reason why the files were originally separated like this
and I can't see anything obvious. I assume you also found no reason.
Presumably the separation was just to avoid disturbing the 32-bit configs
but I think it is a sensible move to merge them.
That was also my guess as to why it was done that way.
Post by Matthew Fortune
With those changes can you double check that a default big and default little
endian build pass -EB/-EL respectively by default and are changed when using
-EL/-EB explicitly? There should only be one -E* option passed to the linker
theoretically, unless multiple explicit -E* options are given on the command
line.
I made the syntax changes, removed the '%{!EB:%{!EL:%(endian_spec)}}'
part of GNU_USER_TARGET_LINK_SPEC and double checked that we still pass
-EL or -EB to the linker in all cases.
Post by Matthew Fortune
Otherwise OK (assuming the link specs behave as described above).
Thanks,
Matthew
Thanks for the review, I have gone ahead and checked in the patch with
those changes.

Steve Ellcey
***@mips.com
Steve Ellcey
2014-10-08 17:42:19 UTC
Permalink
Matthew,

I just discovered that I dropped one of the files I changed as part of
my MIPS header file cleanup patch and so it did not get checked in.
This change was part of my testing, I just didn't include it in the
patch I submitted and checked in. Is this (hopefully obvious) change OK
for checkin?

Steve Ellcey
***@mips.com



2014-10-08 Steve Ellcey <***@mips.com>

* config/mips/mti-linux.h (DRIVER_SELF_SPECS): Change
LINUX64_DRIVER_SELF_SPECS to LINUX_DRIVER_SELF_SPECS


diff --git a/gcc/config/mips/mti-linux.h b/gcc/config/mips/mti-linux.h
index 318e981..34b64d6 100644
--- a/gcc/config/mips/mti-linux.h
+++ b/gcc/config/mips/mti-linux.h
@@ -43,4 +43,4 @@ along with GCC; see the file COPYING3. If not see
BASE_DRIVER_SELF_SPECS \
\
/* Use the standard linux specs for everything else. */ \
- LINUX64_DRIVER_SELF_SPECS
+ LINUX_DRIVER_SELF_SPECS
Matthew Fortune
2014-10-08 19:58:37 UTC
Permalink
Post by Steve Ellcey
* config/mips/mti-linux.h (DRIVER_SELF_SPECS): Change
LINUX64_DRIVER_SELF_SPECS to LINUX_DRIVER_SELF_SPECS
OK. I'd agree with obvious here.

Matthew

Loading...