Re: [PATCH, Android] MIPS support

2012-04-17 Thread Maxim Kuvyrkov 
On 5/04/2012, at 10:16 AM, Maxim Kuvyrkov wrote:

 Chao,
 
 Let's take discussion of MIPS changes to gcc-patches@.  Please follow up here.
 
 --
 Maxim Kuvyrkov
 CodeSourcery / Mentor Graphics
 
 On 5/04/2012, at 10:10 AM, Fu, Chao-Ying wrote:
 
 For now, two MIPS changes in gnu-user.h and unwind-dw2-fde-dip.c can be 
 posted for comment.
 (I didn't tested this patch, though.)

You need to test your patches before posting them for review.  Below are a 
couple of comments on your current version.

 After starting to build toolchains for Android with Bionic, we may find new 
 files to
 patch.  Ex: Comment out getpagesize() for bionic.
 
 Any comment?  Thanks a lot!
 
 Regards,
 Chao-ying
 
 Index: gcc/gcc/config/mips/gnu-user.h
 ===
 --- gcc.orig/gcc/config/mips/gnu-user.h  2012-04-03 17:39:50.0 
 -0700
 +++ gcc/gcc/config/mips/gnu-user.h   2012-04-04 14:31:50.804236000 -0700
 @@ -45,8 +45,8 @@ along with GCC; see the file COPYING3.  
 /* A standard GNU/Linux mapping.  On most targets, it is included in
   CC1_SPEC itself by config/linux.h, but mips.h overrides CC1_SPEC
   and provides this hook instead.  */
 -#undef SUBTARGET_CC1_SPEC
 -#define SUBTARGET_CC1_SPEC %{profile:-p}
 +#undef GNU_USER_SUBTARGET_CC1_SPEC
 +#define GNU_USER_SUBTARGET_CC1_SPEC %{profile:-p}
 
 /* -G is incompatible with -KPIC which is the default, so only allow objects
   in the small data section if the user explicitly asks for it.  */
 @@ -54,8 +54,8 @@ along with GCC; see the file COPYING3.  
 #define MIPS_DEFAULT_GVALUE 0
 
 /* Borrowed from sparc/linux.h */
 -#undef LINK_SPEC
 -#define LINK_SPEC \
 +#undef GNU_USER_TARGET_LINK_SPEC
 +#define GNU_USER_TARGET_LINK_SPEC \
 %(endian_spec) \
  %{shared:-shared} \
  %{!shared: \
 @@ -89,8 +89,8 @@ along with GCC; see the file COPYING3.  
 #undef ASM_OUTPUT_REG_PUSH
 #undef ASM_OUTPUT_REG_POP
 
 -#undef LIB_SPEC
 -#define LIB_SPEC \
 +#undef GNU_USER_TARGET_LIB_SPEC
 +#define GNU_USER_TARGET_LIB_SPEC \
 %{pthread:-lpthread} \
 %{shared:-lc} \
 %{!shared: \
 @@ -133,7 +133,34 @@ extern const char *host_detect_local_cpu
  LINUX_DRIVER_SELF_SPECS
 
 /* Similar to standard Linux, but adding -ffast-math support.  */
 -#undef  ENDFILE_SPEC
 -#define ENDFILE_SPEC \
 +#undef  GNU_USER_TARGET_ENDFILE_SPEC
 +#define GNN_USER_TARGET_ENDFILE_SPEC \
  %{Ofast|ffast-math|funsafe-math-optimizations:crtfastmath.o%s} \
   %{shared|pie:crtendS.o%s;:crtend.o%s} crtn.o%s

Above definitions are OK.

 +
 +#undef  LINK_SPEC
 +#define LINK_SPEC   \
 +  LINUX_OR_ANDROID_LD (GNU_USER_TARGET_LINK_SPEC,   \
 +   GNU_USER_TARGET_LINK_SPEC   ANDROID_LINK_SPEC)
 +
 +#undef  SUBTARGET_CC1_SPEC
 +#define SUBTARGET_CC1_SPEC  \
 +  LINUX_OR_ANDROID_CC (GNU_USER_SUBTARGET_CC1_SPEC, \
 +   GNU_USER_SUBTARGET_CC1_SPEC   ANDROID_CC1_SPEC)
 +
 +#undef  CC1PLUS_SPEC
 +#define CC1PLUS_SPEC
 \
 +  LINUX_OR_ANDROID_CC (, ANDROID_CC1PLUS_SPEC)
 +
 +#undef  LIB_SPEC
 +#define LIB_SPEC\
 +  LINUX_OR_ANDROID_LD (GNU_USER_TARGET_LIB_SPEC,\
 +   GNU_USER_TARGET_LIB_SPEC   ANDROID_LIB_SPEC)
 +
 +#undef  STARTFILE_SPEC
 +#define STARTFILE_SPEC  
 \
 +  LINUX_OR_ANDROID_LD (GNU_USER_TARGET_STARTFILE_SPEC, 
 ANDROID_STARTFILE_SPEC)
 +
 +#undef  ENDFILE_SPEC
 +#define ENDFILE_SPEC
 \
 +  LINUX_OR_ANDROID_LD (GNU_USER_TARGET_ENDFILE_SPEC, ANDROID_ENDFILE_SPEC)

The LINUX_OR_ANDROID_* definitions should be moved out of gnu-user.h, as this 
header is used for systems besides Linux, e.g., kFreeBSD and Hurd.  Please move 
these definitions to mips/linux-common.h, which will be a new file, similarly 
as i386 did in http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00944.html .

 Index: gcc/libgcc/unwind-dw2-fde-dip.c
 ===
 --- gcc.orig/libgcc/unwind-dw2-fde-dip.c 2012-04-03 17:07:28.0 
 -0700
 +++ gcc/libgcc/unwind-dw2-fde-dip.c  2012-04-04 14:51:01.338074000 -0700
 @@ -48,8 +48,9 @@
 #include gthr.h
 
 #if !defined(inhibit_libc)  defined(HAVE_LD_EH_FRAME_HDR) \
 - (__GLIBC__  2 || (__GLIBC__ == 2  __GLIBC_MINOR__  2) \
 -|| (__GLIBC__ == 2  __GLIBC_MINOR__ == 2  defined(DT_CONFIG)))
 + ((defined(__BIONIC__)  (defined(mips) || defined(__mips__))) \
 +|| (__GLIBC__  2 || (__GLIBC__ == 2  __GLIBC_MINOR__  2) \
 +|| (__GLIBC__ == 2  __GLIBC_MINOR__ == 2  defined(DT_CONFIG
 # define USE_PT_GNU_EH_FRAME
 #endif

What is this change for?

Thank you,

--
Maxim Kuvyrkov
CodeSourcery / Mentor Graphics

RE: [PATCH, Android] MIPS support

2012-04-17 Thread Fu, Chao-Ying 
Maxim Kuvyrkov wrote:

  
  For now, two MIPS changes in gnu-user.h and 
 unwind-dw2-fde-dip.c can be posted for comment.
  (I didn't tested this patch, though.)
 
 You need to test your patches before posting them for review. 
  Below are a couple of comments on your current version.

  I can test if this patch doesn't break existing MIPS Linux GCC build.

 
  After starting to build toolchains for Android with 
 Bionic, we may find new files to
  patch.  Ex: Comment out getpagesize() for bionic.
  
  Any comment?  Thanks a lot!
  
  Regards,
  Chao-ying
  
  Index: gcc/gcc/config/mips/gnu-user.h
  ===
  --- gcc.orig/gcc/config/mips/gnu-user.h2012-04-03 
 17:39:50.0 -0700
  +++ gcc/gcc/config/mips/gnu-user.h 2012-04-04 
 14:31:50.804236000 -0700
  @@ -45,8 +45,8 @@ along with GCC; see the file COPYING3.  
  /* A standard GNU/Linux mapping.  On most targets, it is 
 included in
CC1_SPEC itself by config/linux.h, but mips.h overrides CC1_SPEC
and provides this hook instead.  */
  -#undef SUBTARGET_CC1_SPEC
  -#define SUBTARGET_CC1_SPEC %{profile:-p}
  +#undef GNU_USER_SUBTARGET_CC1_SPEC
  +#define GNU_USER_SUBTARGET_CC1_SPEC %{profile:-p}
  
  /* -G is incompatible with -KPIC which is the default, so 
 only allow objects
in the small data section if the user explicitly asks for it.  */
  @@ -54,8 +54,8 @@ along with GCC; see the file COPYING3.  
  #define MIPS_DEFAULT_GVALUE 0
  
  /* Borrowed from sparc/linux.h */
  -#undef LINK_SPEC
  -#define LINK_SPEC \
  +#undef GNU_USER_TARGET_LINK_SPEC
  +#define GNU_USER_TARGET_LINK_SPEC \
  %(endian_spec) \
   %{shared:-shared} \
   %{!shared: \
  @@ -89,8 +89,8 @@ along with GCC; see the file COPYING3.  
  #undef ASM_OUTPUT_REG_PUSH
  #undef ASM_OUTPUT_REG_POP
  
  -#undef LIB_SPEC
  -#define LIB_SPEC \
  +#undef GNU_USER_TARGET_LIB_SPEC
  +#define GNU_USER_TARGET_LIB_SPEC \
  %{pthread:-lpthread} \
  %{shared:-lc} \
  %{!shared: \
  @@ -133,7 +133,34 @@ extern const char *host_detect_local_cpu
   LINUX_DRIVER_SELF_SPECS
  
  /* Similar to standard Linux, but adding -ffast-math support.  */
  -#undef  ENDFILE_SPEC
  -#define ENDFILE_SPEC \
  +#undef  GNU_USER_TARGET_ENDFILE_SPEC
  +#define GNN_USER_TARGET_ENDFILE_SPEC \
   %{Ofast|ffast-math|funsafe-math-optimizations:crtfastmath.o%s} \
%{shared|pie:crtendS.o%s;:crtend.o%s} crtn.o%s
 
 Above definitions are OK.

  Thanks!

 
  +
  +#undef  LINK_SPEC
  +#define LINK_SPEC 
   \
  +  LINUX_OR_ANDROID_LD (GNU_USER_TARGET_LINK_SPEC, 
   \
  + GNU_USER_TARGET_LINK_SPEC   ANDROID_LINK_SPEC)
  +
  +#undef  SUBTARGET_CC1_SPEC
  +#define SUBTARGET_CC1_SPEC
   \
  +  LINUX_OR_ANDROID_CC (GNU_USER_SUBTARGET_CC1_SPEC,   
   \
  + GNU_USER_SUBTARGET_CC1_SPEC   ANDROID_CC1_SPEC)
  +
  +#undef  CC1PLUS_SPEC
  +#define CC1PLUS_SPEC  
   \
  +  LINUX_OR_ANDROID_CC (, ANDROID_CC1PLUS_SPEC)
  +
  +#undef  LIB_SPEC
  +#define LIB_SPEC  
   \
  +  LINUX_OR_ANDROID_LD (GNU_USER_TARGET_LIB_SPEC,  
   \
  + GNU_USER_TARGET_LIB_SPEC   ANDROID_LIB_SPEC)
  +
  +#undef  STARTFILE_SPEC
  +#define STARTFILE_SPEC
   \
  +  LINUX_OR_ANDROID_LD (GNU_USER_TARGET_STARTFILE_SPEC, 
 ANDROID_STARTFILE_SPEC)
  +
  +#undef  ENDFILE_SPEC
  +#define ENDFILE_SPEC  
   \
  +  LINUX_OR_ANDROID_LD (GNU_USER_TARGET_ENDFILE_SPEC, 
 ANDROID_ENDFILE_SPEC)
 
 The LINUX_OR_ANDROID_* definitions should be moved out of 
 gnu-user.h, as this header is used for systems besides Linux, 
 e.g., kFreeBSD and Hurd.  Please move these definitions to 
 mips/linux-common.h, which will be a new file, similarly as 
 i386 did in http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00944.html .

  I will check this message.

 
  Index: gcc/libgcc/unwind-dw2-fde-dip.c
  ===
  --- gcc.orig/libgcc/unwind-dw2-fde-dip.c   2012-04-03 
 17:07:28.0 -0700
  +++ gcc/libgcc/unwind-dw2-fde-dip.c2012-04-04 
 14:51:01.338074000 -0700
  @@ -48,8 +48,9 @@
  #include gthr.h
  
  #if !defined(inhibit_libc)  defined(HAVE_LD_EH_FRAME_HDR) \
  - (__GLIBC__  2 || (__GLIBC__ == 2  __GLIBC_MINOR__  2) \
  -  || (__GLIBC__ == 2  __GLIBC_MINOR__ == 2  
 defined(DT_CONFIG)))
  + ((defined(__BIONIC__)  (defined(mips) || 
 defined(__mips__))) \
  +|| (__GLIBC__  2 || (__GLIBC__ == 2  
 __GLIBC_MINOR__  2) \
  +  || (__GLIBC__ == 2  __GLIBC_MINOR__ == 2  
 defined(DT_CONFIG
  # define USE_PT_GNU_EH_FRAME
  #endif
 
 What is this change for?

  For stack unwinding, MIPS needs supporting functions in libgcc to 
work with eh_frame for Android.
(Note that ARM has its own

Re: [PATCH, Android] MIPS support

2012-04-17 Thread Maxim Kuvyrkov 
On 18/04/2012, at 1:10 PM, Fu, Chao-Ying wrote:

 Maxim Kuvyrkov wrote:
 
 Above definitions are OK.
 
  Thanks!

For avoidance of doubt, please wait for the whole patch to be approved before 
committing it.

 Index: gcc/libgcc/unwind-dw2-fde-dip.c
 ===
 --- gcc.orig/libgcc/unwind-dw2-fde-dip.c   2012-04-03 
 17:07:28.0 -0700
 +++ gcc/libgcc/unwind-dw2-fde-dip.c2012-04-04 
 14:51:01.338074000 -0700
 @@ -48,8 +48,9 @@
 #include gthr.h
 
 #if !defined(inhibit_libc)  defined(HAVE_LD_EH_FRAME_HDR) \
 - (__GLIBC__  2 || (__GLIBC__ == 2  __GLIBC_MINOR__  2) \
 -  || (__GLIBC__ == 2  __GLIBC_MINOR__ == 2  
 defined(DT_CONFIG)))
 + ((defined(__BIONIC__)  (defined(mips) || 
 defined(__mips__))) \
 +|| (__GLIBC__  2 || (__GLIBC__ == 2  
 __GLIBC_MINOR__  2) \
 +  || (__GLIBC__ == 2  __GLIBC_MINOR__ == 2  
 defined(DT_CONFIG
 # define USE_PT_GNU_EH_FRAME
 #endif
 
 What is this change for?
 
  For stack unwinding, MIPS needs supporting functions in libgcc to 
 work with eh_frame for Android.
 (Note that ARM has its own unwinding functions in gcc/config/arm/.  It 
 doesn't use eh_frame.)
 The file is enabled for GLIBC originally.  Thus, I add a new test to enable it
 for MIPS Android BIONIC build.

Please use format that other C libraries use (instead of mixing together GLIBC 
and Bionic definitions):

#if !defined(inhibit_libc)  defined(HAVE_LD_EH_FRAME_HDR) \
 defined(__BIONIC__)
# define USE_PT_GNU_EH_FRAME
#endif

Also, as far as I can tell, this change would also apply for x86, and for ARM 
having USE_PT_GNU_EH_FRAME defined will not hurt.  So please make the 
definition architecture-agnostic.

Thank you,

--
Maxim Kuvyrkov
CodeSourcery / Mentor Graphics