diff mbox

[mips] New mips triplet for multilib linux builds

Message ID d278a1f2-4560-445f-b9e9-50835163ea03@EXCHHUB01.MIPS.com
State New
Headers show

Commit Message

Steve Ellcey Sept. 4, 2012, 9:17 p.m. UTC 
I would like to create a new mips target triplet (mips-mti-linux-gnu).
This target would be multilib by default and would have --enable-synci
on by default.  It would mainly be used for building mips cross compilers
with glibc.  I hope to extend this target to support the n32 and 64 bit
ABIs in the future and add a corresponding mips-mti-elf triplet that would
be like mips-sde-elf but have fewer/different multilib versions.

Other then adding the new target the only changes are to the --enable-synci
default setting (enabled for mips-mti-linux-gnu, still disabled for other
targets) and in mips.h to use a new macro SYNCI_SPEC so that I don't have
to copy all of OPTION_DEFAULT_SPECS into mti-linux.h just to change the
-msynci handling.

I tested the changes by building and running the testsuite with the qemu
simulator.  No glibc or binutils changes were needed for this.

OK to checkin?

Steve Ellcey
sellcey@mips.com


2012-09-04  Steve Ellcey  <sellcey@mips.com>

	* config.gcc: Add mips*-mti-linux* target and make with_synci
	true by default for that target.
	* config/mips/mips.h (SYNCI_SPEC): New.
	(OPTION_DEFAULT_SPECS): Use new SYNCI_SPEC.
	* mti-linux.h: New file.
	* t-mti-linux: New file.

Comments

Richard Sandiford Sept. 4, 2012, 10:55 p.m. UTC | #1 
"Steve Ellcey " <sellcey@mips.com> writes:
> --- a/gcc/config/mips/mips.h
> +++ b/gcc/config/mips/mips.h
> @@ -748,6 +748,9 @@ struct mips_cpu_info {
>       specified.
>     --with-divide is ignored if -mdivide-traps or -mdivide-breaks are
>       specified. */
> +#ifndef SYNCI_SPEC
> +#define SYNCI_SPEC "-m%(VALUE)"
> +#endif
>  #define OPTION_DEFAULT_SPECS \
>    {"arch", "%{" MIPS_ARCH_OPTION_SPEC ":;: -march=%(VALUE)}" }, \
>    {"arch_32", "%{" OPT_ARCH32 ":%{" MIPS_ARCH_OPTION_SPEC ":;: -march=%(VALUE)}}" }, \
> @@ -760,7 +763,7 @@ struct mips_cpu_info {
>    {"divide", "%{!mdivide-traps:%{!mdivide-breaks:-mdivide-%(VALUE)}}" }, \
>    {"llsc", "%{!mllsc:%{!mno-llsc:-m%(VALUE)}}" }, \
>    {"mips-plt", "%{!mplt:%{!mno-plt:-m%(VALUE)}}" }, \
> -  {"synci", "%{!msynci:%{!mno-synci:-m%(VALUE)}}" }
> +  {"synci", "%{!msynci:%{!mno-synci:" SYNCI_SPEC "}}" }
>  
>  
>  /* A spec that infers the -mdsp setting from an -march argument.  */
> diff --git a/gcc/config/mips/mti-linux.h b/gcc/config/mips/mti-linux.h
> new file mode 100644
> index 0000000..af3d71f
> --- /dev/null
> +++ b/gcc/config/mips/mti-linux.h
> @@ -0,0 +1,35 @@
> +/* Target macros for mips*-mti-linux* targets.
> +   Copyright (C) 2012
> +   Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +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.
> +
> +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/>.  */
> +
> +/* Use the (o)32 ABI and the mips32r2 architecture by default.  */
> +#undef MIPS_ABI_DEFAULT
> +#define MIPS_ABI_DEFAULT ABI_32
> +#undef MIPS_ISA_DEFAULT
> +#define MIPS_ISA_DEFAULT 33
> +
> +/* If -msynci/-mno-synci is not specified, default to -msynci on architectures
> +   that support it and -mno-synci on architectures that do not.  */
> +#undef SYNCI_SPEC
> +#define SYNCI_SPEC "%{!mips32:%{!mips64:-m%(VALUE)}}"

One drawback of this is that it won't cope with -march=4kf, etc.
AFAICT, the configuration also won't pick the soft-float multilib
for things like -march=4kc.

I think the configuration should define:

#undef DRIVER_SELF_SPECS
#define DRIVER_SELF_SPECS						\
  /* Make sure a -mips option is present.  This helps us to pick	\
     the right multilib, and also makes the later specs easier		\
     to write.  */							\
  MIPS_ISA_LEVEL_SPEC,							\
									\
  /* Infer the default float setting from -march.  */			\
  MIPS_ARCH_FLOAT_SPEC,							\

(which is boilerplate) so that the right multilibs get selected.
The "problem" (though not really, see below) is that DRIVER_SELF_SPECS
are applied after OPTION_DEFAULT_SPECS, so this won't help with SYNCI_SPEC.

I hadn't realised until reading your patch that the --with-synci
handling is inconsistent with the other options.  It should be:

		case ${with_synci} in
		yes)
			with_synci=synci
			;;
		no)
			with_synci=no-synci
			;;
		"")
			;;

so that the default configure behaviour is not to override the setting
either way.  If we do that, then your DRIVER_SELF_SPECS can further have:

  MIPS_ISA_SYNCI_SPEC

where the definition:

/* Infer a -msynci setting from a -mips argument, on the assumption that
   -msynci is desired where possible.  */
#define MIPS_ISA_SYNCI_SPEC \
  "%{msynci|mno-synci:;%{mips32r2|mips64r2:-msynci:-mno-synci}}"

can go in mips.h.  OPTION_DEFAULT_SPECS would then handle synci in just
the same way as the other options, without the special SYNCI_SPEC macro.

You'll probably want to replace:

#undef DRIVER_SELF_SPECS
#define DRIVER_SELF_SPECS \
  BASE_DRIVER_SELF_SPECS, \
  LINUX_DRIVER_SELF_SPECS \
  " %{!EB:%{!EL:%(endian_spec)}}" \
  " %{!mabi=*: -" MULTILIB_ABI_DEFAULT "}"

with:

#define LINUX64_DRIVER_SELF_SPECS \
  LINUX_DRIVER_SELF_SPECS \
  " %{!EB:%{!EL:%(endian_spec)}}" \
  " %{!mabi=*: -" MULTILIB_ABI_DEFAULT "}"

#undef DRIVER_SELF_SPECS
#define DRIVER_SELF_SPECS \
  BASE_DRIVER_SELF_SPECS, \
  LINUX64_DRIVER_SELF_SPECS

in gnu-user64.h, so that you can reuse LINUX64_DRIVER_SELF_SPECS
(and BASE_DRIVER_SELF_SPECS) in your DRIVER_SELF_SPECS.
LINUX_* is really a misnomer here, but let's leave that for
a later clean-up...

Could you check whether a --with-arch=mips64 --with-float=hard build
does something sensible?  Doesn't need to be a full test, just a minimal-
language build and a bit of hand testing.  This triplet is more elaborate
than the current sysrooted MIPS ones, so I just wanted to make sure that
we really don't need to add the default mips32r2 & mhard-float options
to the MULTILIB_OPTIONS line.  (TBH, I thought we would, although it
wouldn't affect the default configuration.)

Looks good apart from those two concerns.

Richard
Steve Ellcey Sept. 5, 2012, 9:28 p.m. UTC | #2 
On Tue, 2012-09-04 at 23:55 +0100, Richard Sandiford wrote:


> If we do that, then your DRIVER_SELF_SPECS can further have:
> 
>   MIPS_ISA_SYNCI_SPEC
> 
> where the definition:
> 
> /* Infer a -msynci setting from a -mips argument, on the assumption that
>    -msynci is desired where possible.  */
> #define MIPS_ISA_SYNCI_SPEC \
>   "%{msynci|mno-synci:;%{mips32r2|mips64r2:-msynci:-mno-synci}}"
> 
> can go in mips.h.  OPTION_DEFAULT_SPECS would then handle synci in just
> the same way as the other options, without the special SYNCI_SPEC macro.

I am having trouble with this part.  The newly built compiler is choking
on this config spec when building libgcc and I am not sure how to read
it.

I tried looking in gcc/doc to find a description of the spec syntax but
I couldn't find where it was documented.  I don't know what the
semicolon does and I have never seen a 3 part spec like

%{mips32r2|mips64r2:-msynci:-mno-synci}

Is this an 'if-then-else' usage?  I have only ever seen two part usages
like:

%{mips32r2|mips64r2:-msynci}


Steve Ellcey
sellcey@mips.com
Richard Sandiford Sept. 6, 2012, 5:47 a.m. UTC | #3 
Steve Ellcey <sellcey@mips.com> writes:
> On Tue, 2012-09-04 at 23:55 +0100, Richard Sandiford wrote:
>> If we do that, then your DRIVER_SELF_SPECS can further have:
>> 
>>   MIPS_ISA_SYNCI_SPEC
>> 
>> where the definition:
>> 
>> /* Infer a -msynci setting from a -mips argument, on the assumption that
>>    -msynci is desired where possible.  */
>> #define MIPS_ISA_SYNCI_SPEC \
>>   "%{msynci|mno-synci:;%{mips32r2|mips64r2:-msynci:-mno-synci}}"
>> 
>> can go in mips.h.  OPTION_DEFAULT_SPECS would then handle synci in just
>> the same way as the other options, without the special SYNCI_SPEC macro.
>
> I am having trouble with this part.  The newly built compiler is choking
> on this config spec when building libgcc and I am not sure how to read
> it.
>
> I tried looking in gcc/doc to find a description of the spec syntax but
> I couldn't find where it was documented.  I don't know what the
> semicolon does and I have never seen a 3 part spec like
>
> %{mips32r2|mips64r2:-msynci:-mno-synci}
>
> Is this an 'if-then-else' usage?

Yeah, but I typoed, sorry.  It should be:

%{mips32r2|mips64r2:-msynci;:-mno-synci}

Richard
Steve Ellcey Sept. 7, 2012, 12:45 a.m. UTC | #4 
On Thu, 2012-09-06 at 06:47 +0100, Richard Sandiford wrote:

> > Is this an 'if-then-else' usage?
> 
> Yeah, but I typoed, sorry.  It should be:
> 
> %{mips32r2|mips64r2:-msynci;:-mno-synci}
> 
> Richard

OK, I got that working now.  I am still having some issues though.  My
original patch was setup to include mti-linux.h before mips.h and I
think that is good for setting MIPS_ABI_DEFAULT and MIPS_ISA_DEFAULT
because mips.h is going to look and see if those values are set.

But now that I am setting DRIVER_SELF_SPECS in the header it seems
like I should include it after mips.h (so I can override the setting
of DRIVER_SELF_SPECS in mips.h).

Do I need two header files?  One to include before mips.h and one to
include after mips.h?

Steve Ellcey
sellcey@mips.com
Richard Sandiford Sept. 7, 2012, 6:52 a.m. UTC | #5 
Steve Ellcey <sellcey@mips.com> writes:
> On Thu, 2012-09-06 at 06:47 +0100, Richard Sandiford wrote:
>> > Is this an 'if-then-else' usage?
>> 
>> Yeah, but I typoed, sorry.  It should be:
>> 
>> %{mips32r2|mips64r2:-msynci;:-mno-synci}
>> 
>> Richard
>
> OK, I got that working now.  I am still having some issues though.  My
> original patch was setup to include mti-linux.h before mips.h and I
> think that is good for setting MIPS_ABI_DEFAULT and MIPS_ISA_DEFAULT
> because mips.h is going to look and see if those values are set.
>
> But now that I am setting DRIVER_SELF_SPECS in the header it seems
> like I should include it after mips.h (so I can override the setting
> of DRIVER_SELF_SPECS in mips.h).
>
> Do I need two header files?  One to include before mips.h and one to
> include after mips.h?

MIPS_ABI_DEFAULT and MIPS_ISA_DEFAULT are better set in config.gcc.
That also allows you to handle (say) mipsisa32-mti-linux-gnu vs.
mipsisa64-mti-linux-gnu.

I think in general we want more specific header files to come after
less specific ones, so that the more specific ones can override
whatever they like.  E.g. the order for the generic config/ *.hs
is "elfos.h gnu-user.h linux.h" and the order for the MIPS ones
is "mips.h gnu-user.h gnu-user64.h linux64.h".  linux-common.h
coming after linux64.h is an odd-one-out really.

Richard
diff mbox

Patch

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 9ec8a41..6923211 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1685,6 +1685,13 @@  mips*-*-netbsd*)			# NetBSD/mips, either endian.
 	tm_file="elfos.h ${tm_file} mips/elf.h netbsd.h netbsd-elf.h mips/netbsd.h"
 	extra_options="${extra_options} netbsd.opt netbsd-elf.opt"
 	;;
+mips*-mti-linux*)
+	tm_file="dbxelf.h elfos.h gnu-user.h linux.h glibc-stdint.h mips/mti-linux.h ${tm_file} mips/gnu-user.h mips/gnu-user64.h mips/linux64.h mips/linux-common.h"
+	tmake_file="${tmake_file} mips/t-mti-linux"
+	gnu_ld=yes
+	gas=yes
+	test x$with_llsc != x || with_llsc=yes
+	;;
 mips64*-*-linux* | mipsisa64*-*-linux*)
 	tm_file="dbxelf.h elfos.h gnu-user.h linux.h glibc-stdint.h ${tm_file} mips/gnu-user.h mips/gnu-user64.h mips/linux64.h mips/linux-common.h"
 	tmake_file="${tmake_file} mips/t-linux64"
@@ -3262,10 +3269,19 @@  case "${target}" in
 		yes)
 			with_synci=synci
 			;;
-		"" | no)
-			# No is the default.
+		no)
 			with_synci=no-synci
 			;;
+		"")
+			case "${target}" in
+				mips*-mti-*)
+					with_synci=synci
+					;;
+				*)
+					with_synci=no-synci
+					;;
+			esac
+			;;
 		*)
 			echo "Unknown synci type used in --with-synci" 1>&2
 			exit 1
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 9ce466d..b98b434 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -748,6 +748,9 @@  struct mips_cpu_info {
      specified.
    --with-divide is ignored if -mdivide-traps or -mdivide-breaks are
      specified. */
+#ifndef SYNCI_SPEC
+#define SYNCI_SPEC "-m%(VALUE)"
+#endif
 #define OPTION_DEFAULT_SPECS \
   {"arch", "%{" MIPS_ARCH_OPTION_SPEC ":;: -march=%(VALUE)}" }, \
   {"arch_32", "%{" OPT_ARCH32 ":%{" MIPS_ARCH_OPTION_SPEC ":;: -march=%(VALUE)}}" }, \
@@ -760,7 +763,7 @@  struct mips_cpu_info {
   {"divide", "%{!mdivide-traps:%{!mdivide-breaks:-mdivide-%(VALUE)}}" }, \
   {"llsc", "%{!mllsc:%{!mno-llsc:-m%(VALUE)}}" }, \
   {"mips-plt", "%{!mplt:%{!mno-plt:-m%(VALUE)}}" }, \
-  {"synci", "%{!msynci:%{!mno-synci:-m%(VALUE)}}" }
+  {"synci", "%{!msynci:%{!mno-synci:" SYNCI_SPEC "}}" }
 
 
 /* A spec that infers the -mdsp setting from an -march argument.  */
diff --git a/gcc/config/mips/mti-linux.h b/gcc/config/mips/mti-linux.h
new file mode 100644
index 0000000..af3d71f
--- /dev/null
+++ b/gcc/config/mips/mti-linux.h
@@ -0,0 +1,35 @@ 
+/* Target macros for mips*-mti-linux* targets.
+   Copyright (C) 2012
+   Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+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.
+
+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/>.  */
+
+/* Use the (o)32 ABI and the mips32r2 architecture by default.  */
+#undef MIPS_ABI_DEFAULT
+#define MIPS_ABI_DEFAULT ABI_32
+#undef MIPS_ISA_DEFAULT
+#define MIPS_ISA_DEFAULT 33
+
+/* If -msynci/-mno-synci is not specified, default to -msynci on architectures
+   that support it and -mno-synci on architectures that do not.  */
+#undef SYNCI_SPEC
+#define SYNCI_SPEC "%{!mips32:%{!mips64:-m%(VALUE)}}"
+
+/* This target is a multilib target, specify the sysroot paths.  */
+#undef SYSROOT_SUFFIX_SPEC
+#define SYSROOT_SUFFIX_SPEC \
+    "%{mips32:/mips32}%{mips64:/mips64}%{mips64r2:/mips64r2}%{msoft-float:/sof}%{mel|EL:/el}%{mabi=64:/64}%{mabi=n32:/n32}"
diff --git a/gcc/config/mips/t-mti-linux b/gcc/config/mips/t-mti-linux
new file mode 100644
index 0000000..ba11706
--- /dev/null
+++ b/gcc/config/mips/t-mti-linux
@@ -0,0 +1,24 @@ 
+# Copyright (C) 2012 Free Software Foundation, Inc.
+#
+# This file is part of GCC.
+#
+# 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.
+#
+# 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/>.
+
+# The default build is mips32r2, hard-float big-endian.  Add mips32,
+# soft-float, and little-endian variations.
+
+MULTILIB_OPTIONS = mips32/mips64/mips64r2 msoft-float EL
+MULTILIB_DIRNAMES = mips32 mips64 mips64r2 sof el
+MULTILIB_MATCHES = EL=mel EB=meb