[Glibc-bsd-hackers] Re: [PATCH] GNU/k*BSD support (with ChangeLog) for gcc

[Jim Wilson]

Robert Millan wrote:
>         * libtool.m4: Add kfreebsd*-gnu and knetbsd*-gnu.
>         * ltconfig: Likewise.
>         * ltcf-c.sh: Likewise.
>         * ltcf-cxx.sh: Likewise.
>         * ltcf-gcj.sh: Likewise.

These files are from the upstream package libtool.  Our rules say that 
the patches have to be submitted to and accepted by the libtool 
maintainers first.  Once that is done, they can be merged into the gcc 
tree without further approval.

> diff -Nur gcc.old/gcc/config/i386/linux.h gcc/gcc/config/i386/linux.h
> --- gcc.old/gcc/config/i386/linux.h     2003-11-29 04:08:10.000000000 +0100
> +++ gcc/gcc/config/i386/linux.h 2004-03-22 20:45:45.000000000 +0100
> +#undef  SUBTARGET_EXTRA_SPECS  /* i386.h bogusly defines it.  */

The definition in i386.h isn't bogus.  It has to be there. 
SUBTARGET_EXTRA_SPECS is an optional macro.  An OS port defines it only 
if it needs it.  If an OS port doesn't need it, then we use the default 
definition in i386.h.

> +/* Unify sc_* declarations on all platforms */
> +#ifndef sc_eax
> +#define sc_eax eax
> +#define sc_ebx ebx
> +#define sc_ecx ecx
> +#define sc_edx edx
> +#define sc_esi esi
> +#define sc_edi edi
> +#define sc_ebp ebp
> +#define sc_eip eip
> +#endif

This seems unnecessarily wordy.  Couldn't you just have something like
#define REG_NAME(reg) reg

and then in the k*bsd-gnu files this becomes something like
#define REG_NAME (reg) sc_ ## reg
This might have to be written differently, but there should be some way 
to use token pasting here.


> diff -Nur gcc.old/gcc/config/kfreebsd-gnu.h gcc/gcc/config/kfreebsd-gnu.h
> --- gcc.old/gcc/config/kfreebsd-gnu.h   1970-01-01 01:00:00.000000000 +0100
> +++ gcc/gcc/config/kfreebsd-gnu.h       2004-03-22 20:46:33.000000000 +0100

> +#define LINK_EMULATION "elf_i386_fbsd"

This can't be right.  This is a target file that should apply to 
kfreebsd-gnu for all architectures, but this is an obvious i386 
dependence.  Hence it should be in a config/i386/kfreebsd-gnu.h file. 
Also, it is overriding a macro that only exists in config/i386/linux.h, 
and hence it makes no sense to put it in a target independent config 
file.  Same this for DYNAMIC_LINKER which is i386 specific macro.

This is a problem you have inherited from the current kfreebsdgnu.h file 
which has the same problem.

> +/* These must be macros so that i386/linux.h can check for them */
> +#define sc_eax sc_eax

Again, you have architecture dependent stuff in an architecture 
independent file.  This should instead be in a 
config/i386/kfreebsd-gnu.h file.


> diff -Nur gcc.old/gcc/config/knetbsd-gnu.h gcc/gcc/config/knetbsd-gnu.h
> --- gcc.old/gcc/config/knetbsd-gnu.h    1970-01-01 01:00:00.000000000 +0100
> +++ gcc/gcc/config/knetbsd-gnu.h        2004-03-22 20:46:51.000000000 +0100

> +#define DYNAMIC_LINKER "/lib/ld.so.1"
> +/* These must be macros so that i386/linux.h can check for them */
> +#define sc_eax sc_eax

Same as above.  You shouldn't have architecture dependent macros in an 
architecture independent file.

> diff -Nur gcc.old/gcc/config.gcc gcc/gcc/config.gcc
> --- gcc.old/gcc/config.gcc      2004-03-14 18:24:36.000000000 +0100
> +++ gcc/gcc/config.gcc  2004-03-22 20:32:27.000000000 +0100

> -       tm_file="${tm_file} i386/unix.h i386/att.h dbxelf.h elfos.h svr4.h linux.h 
> i386/linux.h"
> +       tm_file="${tm_file} i386/unix.h i386/att.h dbxelf.h elfos.h svr4.h"
> +       case ${target} in
> +       i[34567]86-*-knetbsd*-gnu) tm_file="${tm_file} knetbsd-gnu.h" ;;
> +       i[34567]86-*-kfreebsd*-gnu) tm_file="${tm_file} kfreebsd-gnu.h" ;;
> +       esac
> +       tm_file="${tm_file} linux.h i386/linux.h"

I think you made the wrong choice here.  kfreebsd-gnu.h and 
knetbsd-gnu.h should come after linux.h instead of before it, since you 
are trying to override macros defined in the linux.h files.

With this change, the patch gets simpler.  You no longer need any 
config/linux.h change at all.  You no longer need the #ifndef changes in 
the config/i386/linux.h file.  This makes the patch friendlier to the 
linux maintainers.

This does mean a few extra #undef need to be added to the kfreebsd-gnu.h 
and knetbsd-gnu.h files, but that just helps make it obvious that we are 
overriding linux defaults.
--
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com