Re: [patch, mips] New mips triplet for multilib linux builds

[Richard Sandiford <rdsandiford at googlemail dot com>]

"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