[PATCH, Android] MIPS support

[PATCH, Android] MIPS support

[Maxim Kuvyrkov]

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:

> Maxim Kuvyrkov wrote:
> 
>> I encourage you to submit the MIPS Android patches to 
>> gcc-patches@.  And, as long as your changes preserve the 
>> status quo of mips-*-* being big-endian by default and 
>> mipsel-*-* being little-endian by default, there should be no 
>> major obstacles to merge those in.
>> 
> 
>  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.)
> 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.000000000 -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"
> +
> +#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)
> Index: gcc/libgcc/unwind-dw2-fde-dip.c
> ===================================================================
> --- gcc.orig/libgcc/unwind-dw2-fde-dip.c	2012-04-03 17:07:28.000000000 -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
> 

Re: [PATCH, Android] MIPS support

[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.000000000 -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.000000000 -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

[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.h	2012-04-03 
> 17:39:50.000000000 -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.000000000 -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?

  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.

  Thanks!

Regards,
Chao-ying

Re: [PATCH, Android] MIPS support

[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.000000000 -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?
> 
>  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