Re: [PATCH #3] mips*-kfreebsd-gnu support in GAS

[Richard Sandiford <rdsandiford at googlemail dot com>]

Robert Millan <rmh@gnu.org> writes:
> diff -Nur binutils/gas/config/tc-mips.c binutils.gas/gas/config/tc-mips.c
> --- binutils/gas/config/tc-mips.c	2010-12-10 15:25:04.000000000 +0100
> +++ binutils.gas/gas/config/tc-mips.c	2010-12-14 17:09:54.000000000 +0100
> @@ -1333,13 +1333,13 @@
>        /* This is traditional mips.  */
>        return (target_big_endian
>  	      ? (HAVE_64BIT_OBJECTS
> -		 ? "elf64-tradbigmips"
> +		 ? TARGET_SUFFIX ("elf64-tradbigmips")
>  		 : (HAVE_NEWABI
> -		    ? "elf32-ntradbigmips" : "elf32-tradbigmips"))
> +		    ? TARGET_SUFFIX ("elf32-ntradbigmips") : TARGET_SUFFIX ("elf32-tradbigmips")))
>  	      : (HAVE_64BIT_OBJECTS
> -		 ? "elf64-tradlittlemips"
> +		 ? TARGET_SUFFIX ("elf64-tradlittlemips")
>  		 : (HAVE_NEWABI
> -		    ? "elf32-ntradlittlemips" : "elf32-tradlittlemips")));
> +		    ? TARGET_SUFFIX ("elf32-ntradlittlemips") : TARGET_SUFFIX ("elf32-tradlittlemips"))));

Watch the long lines (limit is 80 characters).  However, rather than
do this just for the TE_TMIPS case, and have...

> diff -Nur binutils/gas/config/te-tmips-freebsd.h binutils.gas/gas/config/te-tmips-freebsd.h
> --- binutils/gas/config/te-tmips-freebsd.h	1970-01-01 01:00:00.000000000 +0100
> +++ binutils.gas/gas/config/te-tmips-freebsd.h	2010-12-14 17:09:54.000000000 +0100
> @@ -0,0 +1,27 @@
> +/* te-tmips-freebsd.h -- Traditional MIPS / FreeBSD target environment declarations.
> +   Copyright 2000, 2005, 2007 Free Software Foundation, Inc.
> +
> +   This file is part of GAS, the GNU Assembler.
> +
> +   GAS 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.
> +
> +   GAS 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 GAS; see the file COPYING.  If not, write to the Free
> +   Software Foundation, 51 Franklin Street - Fifth Floor, Boston, MA
> +   02110-1301, USA.  */
> +
> +/* Target environment for FreeBSD.  It is the same as the generic
> +   target, except that it arranges via the TE_FreeBSD define to
> +   suppress the use of "/" as a comment character.  Some code in the
> +   FreeBSD kernel uses "/" to mean division.  (What a concept!)  */
> +#define TE_FreeBSD 1
> +
> +#include "te-tmips.h"

...this extra file, I think it would be cleaner to have:

#if defined (TE_FreeBSD)
#define ELF_TARGET(PREFIX, ENDIAN) PREFIX "trad" ENDIAN "mips-freebsd"
#if defined (TE_TMIPS)
#define ELF_TARGET(PREFIX, ENDIAN) PREFIX "trad" ENDIAN "mips"
#else
#define ELF_TARGET(PREFIX, ENDIAN) PREFIX ENDIAN "mips"
#endif

in tc-mips.c.  The configure scripts could then just use the ordinary
te_freebsd file.  mips_target_format would look like this:

      return (target_big_endian
	      ? (HAVE_64BIT_OBJECTS
		 ? ELF_TARGET ("elf64-", "big")
		 : HAVE_NEWABI
		 ? ELF_TARGET ("elf32-n", "big")
		 : ELF_TARGET ("elf32-", "big"))
	      : (HAVE_64BIT_OBJECTS
		 ? ELF_TARGET ("elf64-", "little")
		 : HAVE_NEWABI
		 ? ELF_TARGET ("elf32-n", "little")
		 : ELF_TARGET ("elf32-", "little")));

and support_64bit_objects would look like this:

  list = bfd_target_list ();
  for (l = list; *l != NULL; l++)
    if (strcmp (*l, ELF_TARGET ("elf64-", "big")) == 0
	|| strcmp (*l, ELF_TARGET ("elf64-", "little")) == 0)
      break;

(all untested).

Richard

PS. Sorry for the delay in reviewing this stuff, been very busy...