This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [arm-linux target] better support for soft-float and default cpu


Hi Jeroen,

  Thanks very much for submitting your patch.  There are a few small
points which need to be fixed, but with those taken care of I believe
that the revisedversion will be acceptable - with one major caveat:
Do you have a gcc copyright assignment on file with the FSF ?  Without
such an assignment we cannot accept your patch :-(

Anyway here are my comments:


> + /* original names for target */

Comment formating:  Start sentences with an uppercase letter.  End
sentences with a period followed by two space.  This applies to several
other comments in the patch too.


> + #if !defined(TARGET_CPU_DEFAULT_NAME) && (TARGET_CPU_DEFAULT == TARGET_CPU_arm2)
> + #define TARGET_CPU_DEFAULT_NAME arm2
> + #endif
> + 
> + #if !defined(TARGET_CPU_DEFAULT_NAME) && (TARGET_CPU_DEFAULT == TARGET_CPU_arm250)
> + #define TARGET_CPU_DEFAULT_NAME arm250
> + #endif
> ....

This whole section could be more efficiently coded as:

  #ifndef TARGET_CPU_DEFAULT_NAME
  #if TARGET_CPU_DEFAULT == TARGET_CPU_arm2
  #define TARGET_CPU_DEFAULT_NAME arm2

  #if TARGET_CPU_DEFAULT == TARGET_CPU_arm250
  #define TARGET_CPU_DEFAULT_NAME arm250
  ....
  #endif


> + #define TARGET_TO_STRING_B(t) # t
> + #define TARGET_TO_STRING(t) TARGET_TO_STRING_B(t)

This is an ANSI-ism.  In theory you ought to also include the K&R
method for stringification, although there are several other places in
the ARM subdirectory where ANSI only code is used, so this is not too
important.

>   #endif
>   
> + 
> + 
>   #ifndef CPP_PREDEFINES

Unnecessary extra spaces.


> + #ifdef TARGET_CPU_DEFAULT_USE_SOFTFLOAT
> + /* we add the library to libgcc */
> + #define LIBGCC_SPEC "-lgcc"
> + #else
>   #define LIBGCC_SPEC "%{msoft-float:-lfloat} -lgcc"
> + #endif

This looks wrong to me.  Surely if the default is for soft float then
this ought to be:

    #ifdef TARGET_CPU_DEFAULT_USE_SOFTFLOAT
    #define LIBGCC_SPEC "%{!mhard-float:-lfloat} -lgcc"
    #else
    #define LIBGCC_SPEC "%{msoft-float:-lfloat} -lgcc"
    #endif

> diff -rNC5 gcc-3.0.2/gcc/config/arm/sflinux-elf.h gcc-3.0.2_new/gcc/config/arm/sflinux-elf.h
> *** gcc-3.0.2/gcc/config/arm/sflinux-elf.h	Thu Jan  1 01:00:00 1970
> --- gcc-3.0.2_new/gcc/config/arm/sflinux-elf.h	Mon Nov 26 12:12:28 2001
> ***************
> *** 0 ****
> --- 1,26 ----
> + /* Definitions for ARM running ucLinux using ELF
> +    Copyright (C) 1999 Free Software Foundation, Inc.

The copyright date ought to be 2001.

> +    Contributed by Philip Blundell <pb@nexus.co.uk>

Since you created this file, you are the contributor.  You could say
that it is based on the file XXX contributed by Philip.

> + This file is part of GNU CC.

GNU CC is now refereed to as GCC.

> + # inhibit libc (for bootstrapping on systems without libc)
> + TARGET_LIBGCC2_CFLAGS += -Dinhibit_libc -D__gthr_posix_h

Why is __gthr_posix_h being defined here ?


Cheers
        Nick


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]