[GCC Steering Committee] Android sub-port reviewer

[GCC Steering Committee] Android sub-port reviewer

[Maxim Kuvyrkov]

I volunteer as the reviewer for Android sub-port.

Android/Bionic support is an extension over Linux port and is being gradually added for more and more architectures.  I wrote the original Android GCC support for ARM (under a watchful design eye of Joseph Myers), and know how the bits fit together.

As adding Android support to a new architecture requires changes to that architecture, the architecture maintainer will have the power of veto for the Android-related changes.

Thank you,

--
Maxim Kuvyrkov
CodeSourcery / Mentor Graphics

Re: [GCC Steering Committee] Android sub-port reviewer

[Maxim Kuvyrkov]

On 29/03/2012, at 5:38 PM, Maxim Kuvyrkov wrote:

> I volunteer as the reviewer for Android sub-port.
> 
> Android/Bionic support is an extension over Linux port and is being gradually added for more and more architectures.  I wrote the original Android GCC support for ARM (under a watchful design eye of Joseph Myers), and know how the bits fit together.
> 
> As adding Android support to a new architecture requires changes to that architecture, the architecture maintainer will have the power of veto for the Android-related changes.

One of the members of SC raised a good point about whether architecture maintainers would prefer to handle the Android patches themselves.  My intention is to spare you the additional headache of dealing with Android, but, well, maybe you like it :-).

Richard E.,
Jan,
Richard S.,

Do you want to handle Android changes by yourself or do you want to delegate?

Thank you,

--
Maxim Kuvyrkov
CodeSourcery / Mentor Graphics

Re: [GCC Steering Committee] Android sub-port reviewer

[Jan Hubicka]

> On 29/03/2012, at 5:38 PM, Maxim Kuvyrkov wrote:
> 
> > I volunteer as the reviewer for Android sub-port.
> > 
> > Android/Bionic support is an extension over Linux port and is being gradually added for more and more architectures.  I wrote the original Android GCC support for ARM (under a watchful design eye of Joseph Myers), and know how the bits fit together.
> > 
> > As adding Android support to a new architecture requires changes to that architecture, the architecture maintainer will have the power of veto for the Android-related changes.
> 
> One of the members of SC raised a good point about whether architecture maintainers would prefer to handle the Android patches themselves.  My intention is to spare you the additional headache of dealing with Android, but, well, maybe you like it :-).
> 
> Richard E.,
> Jan,
> Richard S.,
> 
> Do you want to handle Android changes by yourself or do you want to delegate?

I am happy with the idea of having Android manintainer.  The configure bits are
kind of independent of the actual architecture support anyway.

Honza
> 
> Thank you,
> 
> --
> Maxim Kuvyrkov
> CodeSourcery / Mentor Graphics
> 
> 
> 

Re: [GCC Steering Committee] Android sub-port reviewer

[Richard Earnshaw]

On 01/04/12 20:57, Maxim Kuvyrkov wrote:
> On 29/03/2012, at 5:38 PM, Maxim Kuvyrkov wrote:
> 
>> I volunteer as the reviewer for Android sub-port.
>>
>> Android/Bionic support is an extension over Linux port and is being gradually added for more and more architectures.  I wrote the original Android GCC support for ARM (under a watchful design eye of Joseph Myers), and know how the bits fit together.
>>
>> As adding Android support to a new architecture requires changes to that architecture, the architecture maintainer will have the power of veto for the Android-related changes.
> 
> One of the members of SC raised a good point about whether architecture maintainers would prefer to handle the Android patches themselves.  My intention is to spare you the additional headache of dealing with Android, but, well, maybe you like it :-).
> 
> Richard E.,
> Jan,
> Richard S.,
> 
> Do you want to handle Android changes by yourself or do you want to delegate?
> 
> Thank you,
> 
> --
> Maxim Kuvyrkov
> CodeSourcery / Mentor Graphics
> 
> 
> 
> 
> 
> 

I have no desire to hold back android development, so I have no
objections to an Android-specific maintainer.

But the only file in the ARM back-end that is Android specific is
t-linux-android, a grand total of ten lines of code.

Any file shared with other arm ports would still need generic ARM
maintainer approval.

There are, however, some target independent android files in the config
directory and I guess it would make sense for a maintainer to be
appointed for those.

R.

Re: [GCC Steering Committee] Android sub-port reviewer

[Richard Sandiford]

Maxim Kuvyrkov <maxim@codesourcery.com> writes:
> On 29/03/2012, at 5:38 PM, Maxim Kuvyrkov wrote:
>
>> I volunteer as the reviewer for Android sub-port.
>> 
>> Android/Bionic support is an extension over Linux port and is being gradually added for more and more architectures.  I wrote the original Android GCC support for ARM (under a watchful design eye of Joseph Myers), and know how the bits fit together.
>> 
>> As adding Android support to a new architecture requires changes to that architecture, the architecture maintainer will have the power of veto for the Android-related changes.
>
> One of the members of SC raised a good point about whether architecture maintainers would prefer to handle the Android patches themselves.  My intention is to spare you the additional headache of dealing with Android, but, well, maybe you like it :-).
>
> Richard E.,
> Jan,
> Richard S.,
>
> Do you want to handle Android changes by yourself or do you want to delegate?

Having a separate maintainer of MIPS Android sounds good to me too
(and thanks for asking).  As Richard E says, we should probably coordinate
any changes to the generic MIPS files, but I wouldn't know what's right
for Android-specific stuff.

Richard

RE: [GCC Steering Committee] Android sub-port reviewer

[Fu, Chao-Ying]

Richard Sandiford wrote:
> Sent: Monday, April 02, 2012 11:45 AM
> To: Maxim Kuvyrkov
> Cc: Richard Earnshaw; Jan Hubicka; gcc@gcc.gnu.org
> Subject: Re: [GCC Steering Committee] Android sub-port reviewer
> 
> Maxim Kuvyrkov <maxim@codesourcery.com> writes:
> > On 29/03/2012, at 5:38 PM, Maxim Kuvyrkov wrote:
> >
> >> I volunteer as the reviewer for Android sub-port.
> >> 
> >> Android/Bionic support is an extension over Linux port and 
> is being gradually added for more and more architectures.  I 
> wrote the original Android GCC support for ARM (under a 
> watchful design eye of Joseph Myers), and know how the bits 
> fit together.
> >> 
> >> As adding Android support to a new architecture requires 
> changes to that architecture, the architecture maintainer 
> will have the power of veto for the Android-related changes.
> >
> > One of the members of SC raised a good point about whether 
> architecture maintainers would prefer to handle the Android 
> patches themselves.  My intention is to spare you the 
> additional headache of dealing with Android, but, well, maybe 
> you like it :-).
> >
> > Richard E.,
> > Jan,
> > Richard S.,
> >
> > Do you want to handle Android changes by yourself or do you 
> want to delegate?
> 
> Having a separate maintainer of MIPS Android sounds good to me too
> (and thanks for asking).  As Richard E says, we should 
> probably coordinate
> any changes to the generic MIPS files, but I wouldn't know 
> what's right
> for Android-specific stuff.
> 
> Richard
> 

Hi All,

  We did the MIPS Android port, and here is the change that we submitted to googlesource.
https://android-review.googlesource.com/#/c/34000/

  Please check this file: 0007-gcc-mips.patch under ndk/build/tools/toolchain-patches/gcc after
"git clone https://android.googlesource.com/platform/ndk"
(Or, https://android-review.googlesource.com/#/c/34000/2/build/tools/toolchain-patches/gcc/0007-gcc-mips.patch)

  It basically sets the MIPS target to little-endian MIPS32 for mips-linux-android.
Also, it changes the GCC driver for MIPS Android.
For MIPS Android unwinding supports, we add __BIONIC__ check to compile unwind-dw2-fde-glibc.c.

  We will be glad to help to merge code to GCC mainline.  Thanks!

Regards,
Chao-ying

Re: [GCC Steering Committee] Android sub-port reviewer

[Andrew Pinski]

On Mon, Apr 2, 2012 at 1:55 PM, Fu, Chao-Ying <fu@mips.com> wrote:
>  It basically sets the MIPS target to little-endian MIPS32 for mips-linux-android.

That seems broken because mips-*-* is big-endian and mipsel-*-* is
little-endian.  Is any way of fixing that before even trying to
submitting the patch?  Because it will be the only port for mips which
is the opposite endian which makes it out of place.

Thanks,
Andrew Pinski

RE: [GCC Steering Committee] Android sub-port reviewer

[Fu, Chao-Ying]

Andrew Pinski wrote:

> On Mon, Apr 2, 2012 at 1:55 PM, Fu, Chao-Ying <fu@mips.com> wrote:
> >  It basically sets the MIPS target to little-endian MIPS32 
> for mips-linux-android.
> 
> That seems broken because mips-*-* is big-endian and mipsel-*-* is
> little-endian.  Is any way of fixing that before even trying to
> submitting the patch?  Because it will be the only port for mips which
> is the opposite endian which makes it out of place.

  We fixed the MIPS target to little endian, because there is only one
MIPS ABI in official NDK for Android: little-endian MIPS32 release 1.
NDK scripts are tested with mips-linux-android.  (We didn't test mipsel-linux-android targets.)
Note that Android software components (ex: google v8, webkit jsc) may be coded for
little-endian targets only.

  If GCC doesn't set the MIPS Android targets to little endian, it will be easy to
add a small patch into googlesource in the future.  Ex: Add -EL options for compilation/linking.
Thanks!

Regards,
Chao-ying

  

Re: [GCC Steering Committee] Android sub-port reviewer

[Andrew Pinski]

On Tue, Apr 3, 2012 at 10:45 AM, Fu, Chao-Ying <fu@mips.com> wrote:
> Andrew Pinski wrote:
>
>> On Mon, Apr 2, 2012 at 1:55 PM, Fu, Chao-Ying <fu@mips.com> wrote:
>> >  It basically sets the MIPS target to little-endian MIPS32
>> for mips-linux-android.
>>
>> That seems broken because mips-*-* is big-endian and mipsel-*-* is
>> little-endian.  Is any way of fixing that before even trying to
>> submitting the patch?  Because it will be the only port for mips which
>> is the opposite endian which makes it out of place.
>
>  We fixed the MIPS target to little endian, because there is only one
> MIPS ABI in official NDK for Android: little-endian MIPS32 release 1.
> NDK scripts are tested with mips-linux-android.  (We didn't test mipsel-linux-android targets.)
> Note that Android software components (ex: google v8, webkit jsc) may be coded for
> little-endian targets only.
>
>  If GCC doesn't set the MIPS Android targets to little endian, it will be easy to
> add a small patch into googlesource in the future.  Ex: Add -EL options for compilation/linking.
> Thanks!

The point is mips*-*-* is big endian and mipsel*-*-* is little endian.
 And doing adding a target which says mips-linux-android which is
little-endian is just backwards.  Is there anyway to fix the target
triplet to be mipsel-linux-android including inside the official NDK?
If not then we have a broken triplet for android.

Also it seems broken they are coded for little-endian only but that is
a different point all together.  Also there are a few MIPS64
processors out there which default normally to big-endian (octeon).


Thanks,
Andrew Pinski

RE: [GCC Steering Committee] Android sub-port reviewer

[Fu, Chao-Ying]

Andrew Pinski wrote:
> The point is mips*-*-* is big endian and mipsel*-*-* is little endian.
>  And doing adding a target which says mips-linux-android which is
> little-endian is just backwards.  Is there anyway to fix the target
> triplet to be mipsel-linux-android including inside the official NDK?
> If not then we have a broken triplet for android.

  I agree with what you said.

  The simplest fix is that mips-linux-android still generates big-endian code
as the MIPS target triplet design.  Don't ever set it to generate little-endian code automatically.
For MIPS GCC Android patches to merge to FSF GCC trunk, we will never change endiannes.

  Note that we did use a regular mips-linux-gnu GCC to compile Android systems by using -EL.
The target of "mips-linux-android" is just invented to work with NDK scripts.
And people can freely create "mipsel-linux-*" toolchains to compile Android systems directly,
or create "mips-linux-*" toolchains to compile Android systems with -EL.

  The MIPS NDK script patch was just merged to googlesource last night.
I can work on another patch to use -EL options, but to let
"mips-linux-andoird" generate big-endian code.  Is this solution ok for you?

  Thanks!

Regards,
Chao-ying

Re: [GCC Steering Committee] Android sub-port reviewer

[Maxim Kuvyrkov]

On 4/04/2012, at 6:28 AM, Fu, Chao-Ying wrote:

> Andrew Pinski wrote:
>> The point is mips*-*-* is big endian and mipsel*-*-* is little endian.
>> And doing adding a target which says mips-linux-android which is
>> little-endian is just backwards.  Is there anyway to fix the target
>> triplet to be mipsel-linux-android including inside the official NDK?
>> If not then we have a broken triplet for android.
> 
>  I agree with what you said.
> 
>  The simplest fix is that mips-linux-android still generates big-endian code
> as the MIPS target triplet design.  Don't ever set it to generate little-endian code automatically.
> For MIPS GCC Android patches to merge to FSF GCC trunk, we will never change endiannes.
> 
>  Note that we did use a regular mips-linux-gnu GCC to compile Android systems by using -EL.
> The target of "mips-linux-android" is just invented to work with NDK scripts.
> And people can freely create "mipsel-linux-*" toolchains to compile Android systems directly,
> or create "mips-linux-*" toolchains to compile Android systems with -EL.
> 
>  The MIPS NDK script patch was just merged to googlesource last night.
> I can work on another patch to use -EL options, but to let
> "mips-linux-andoird" generate big-endian code.  Is this solution ok for you?

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.

Thank you,

--
Maxim Kuvyrkov
CodeSourcery / Mentor Graphics

RE: [GCC Steering Committee] Android sub-port reviewer

[Fu, Chao-Ying]

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