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: [PATCH, i386, Android] -mandroid support for i386 target


Hello Maxim,

Thanks a lot for review. My comments are below.

> On 28/02/2012, at 3:41 AM, Ilya Enkovich wrote:
>
>>> You should keep those *_SPEC and define them with new
>>> GNU_*_SPEC in gnu-user.h since gnu-user.h is also used
>>> by other non-linux targets. ?In linux.h, you undef *_SPEC
>>> before defining them.
>>>
>>>
>>> --
>>> H.J.
>>
>> Thanks for the note. Here is fixed version. Is it OK now?
>
> This version looks mostly OK, but still needs a bit of work and testing.
>
> How did you test this patch? ?You should have built before- and after-patch compilers for both Linux and Android and run regression testsuites at least for Linux, and make sure there are no new failures.
>
> As is, it appears this patch did not see much testing, I'm pretty sure it breaks building shared libraries and PIE executable for Linux.

I do not expect any changes in compiler behavior for non Android
targets. I bootstrapped and checked patched compiler on linux-x86_64.
I also built ptched compiler for Android target using NDK scripts.

>
>
>>
>> Thanks,
>> Ilya
>> --
>> 2012-02-27 ?Enkovich Ilya ?<ilya.enkovich@intel.com>
>>
>> ? ? ? * gcc/config/i386/gnu-user.h (GNU_USER_TARGET_CC1_SPEC): New.
>> ? ? ? (CC1_SPEC): Use GNU_USER_TARGET_CC1_SPEC.
>> ? ? ? (GNU_USER_TARGET_LINK_SPEC): New.
>> ? ? ? (LINK_SPEC): Use GNU_USER_TARGET_LINK_SPEC.
>> ? ? ? (GNU_USER_TARGET_MATHFILE_SPEC): New.
>> ? ? ? (ENDFILE_SPEC): Use GNU_USER_TARGET_MATHFILE_SPEC.
>>
>> ? ? ? * gcc/config/i386/linux.h (CC1_SPEC): New.
>> ? ? ? (LINK_SPEC): New.
>> ? ? ? (LIB_SPEC): New.
>> ? ? ? (STARTFILE_SPEC): New.
>> ? ? ? (ENDFILE_SPEC): New.
>>
>>
>> diff --git a/gcc/config/i386/gnu-user.h b/gcc/config/i386/gnu-user.h
>> index 98d0a25..33ceab7 100644
>> --- a/gcc/config/i386/gnu-user.h
>> +++ b/gcc/config/i386/gnu-user.h
>> @@ -77,8 +77,11 @@ along with GCC; see the file COPYING3. ?If not see
>> #undef CPP_SPEC
>> #define CPP_SPEC "%{posix:-D_POSIX_SOURCE} %{pthread:-D_REENTRANT}"
>>
>> +#undef GNU_USER_TARGET_CC1_SPEC
>> +#define GNU_USER_TARGET_CC1_SPEC "%(cc1_cpu) %{profile:-p}"
>
> Here and in other instances below you use "GNU_USER_TARGET_" prefix. ?I would prefer using a shorter "GNU_USER_" instead unless there is a good reason to add "TARGET" too.

The reason is that some macroses are defined in other files and I just
redefine them (i.e. GNU_USER_TARGET_CC1_SPEC). This prefix is also
used for other targets (i.e. in linux-eabi.h). So I do not see a good
reason to change it everywhere or mix it with other prefix
"GNU_USER_".

>
>> +
>> #undef CC1_SPEC
>> -#define CC1_SPEC "%(cc1_cpu) %{profile:-p}"
>> +#define CC1_SPEC GNU_USER_TARGET_CC1_SPEC
>>
>> /* Provide a LINK_SPEC appropriate for GNU userspace. ?Here we provide support
>> ? ?for the special GCC options -static and -shared, which allow us to
>> @@ -97,22 +100,28 @@ along with GCC; see the file COPYING3. ?If not see
>> ? { "link_emulation", GNU_USER_LINK_EMULATION },\
>> ? { "dynamic_linker", GNU_USER_DYNAMIC_LINKER }
>>
>> -#undef ? ? ? LINK_SPEC
>> -#define LINK_SPEC "-m %(link_emulation) %{shared:-shared} \
>> +#define GNU_USER_TARGET_LINK_SPEC \
>> + ?"-m %(link_emulation) %{shared:-shared} \
>> ? %{!shared: \
>> ? ? %{!static: \
>> ? ? ? %{rdynamic:-export-dynamic} \
>> ? ? ? -dynamic-linker %(dynamic_linker)} \
>> ? ? ? %{static:-static}}"
>>
>> +#undef ? ? ? LINK_SPEC
>> +#define LINK_SPEC GNU_USER_TARGET_LINK_SPEC
>> +
>> /* Similar to standard GNU userspace, but adding -ffast-math support. ?*/
>> -#undef ?ENDFILE_SPEC
>> -#define ENDFILE_SPEC \
>> +#define GNU_USER_TARGET_MATHFILE_SPEC \
>> ? "%{Ofast|ffast-math|funsafe-math-optimizations:crtfastmath.o%s} \
>> ? ?%{mpc32:crtprec32.o%s} \
>> ? ?%{mpc64:crtprec64.o%s} \
>> - ? %{mpc80:crtprec80.o%s} \
>> - ? %{shared|pie:crtendS.o%s;:crtend.o%s} crtn.o%s"
>> + ? %{mpc80:crtprec80.o%s}"
>> +
>> +#undef ?ENDFILE_SPEC
>> +#define ENDFILE_SPEC \
>> + ?GNU_USER_TARGET_MATHFILE_SPEC " " \
>> + ?GNU_USER_TARGET_ENDFILE_SPEC
>
> Here you remove "%{shared|pie:crtendS.o%s;:crtend.o%s} crtn.o%s". ?Presumably, you are moving that to GNU_USER[_TARGET]_ENDFILE_SPEC, but you never define it.

You are right. It is in GNU_USER_TARGET_ENDFILE_SPEC which is defined
in gcc/config/gnu-user.h.

>
>>
>> /* A C statement (sans semicolon) to output to the stdio stream
>> ? ?FILE the assembler definition of uninitialized global DECL named
>> diff --git a/gcc/config/i386/linux.h b/gcc/config/i386/linux.h
>> index 73681fe..a832ddc 100644
>> --- a/gcc/config/i386/linux.h
>> +++ b/gcc/config/i386/linux.h
>
> i386/linux.h is used only for simple x86 32-bit builds; i386/linux64.h is used for multilib-enabled x86 toolchains. ?Placing below definitions in i386/linux.h will not allow adding an Android as an additional multilib to -m32/-m64 x86 builds. ?I improve on this situation I suggest:
> - rename i386/linux.h to i386/linux32.h (with corresponding change to config.gcc),
> - put below definitions to new i386/linux.h (remember to add license notice header to the new file),
> - include i386/linux.h from both i386/linux32.h and i386/linux64.h.

Android does not support x86_64 target and therefore I do not want
-mandroid support for this target. When Android supports x86_64 target
this change would be appropriate.

>
>> @@ -22,3 +22,30 @@ along with GCC; see the file COPYING3. ?If not see
>>
>> #define GNU_USER_LINK_EMULATION "elf_i386"
>> #define GLIBC_DYNAMIC_LINKER "/lib/ld-linux.so.2"
>> +
>> +#undef CC1_SPEC
>> +#define CC1_SPEC \
>> + ?LINUX_OR_ANDROID_CC (GNU_USER_TARGET_CC1_SPEC, \
>> + ? ? ? ? ? ? ? ? ? ?GNU_USER_TARGET_CC1_SPEC " " ANDROID_CC1_SPEC)
>> +
>> +#undef ? ? ? LINK_SPEC
>> +#define LINK_SPEC \
>> + ?LINUX_OR_ANDROID_LD (GNU_USER_TARGET_LINK_SPEC, \
>> + ? ? ? ? ? ? ? ? ? ?GNU_USER_TARGET_LINK_SPEC " " ANDROID_LINK_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_MATHFILE_SPEC " " \
>> + ? ? ? ? ? ? ? ? ? ?GNU_USER_TARGET_ENDFILE_SPEC, ? ? \
>> + ? ? ? ? ? ? ? ? ? ?GNU_USER_TARGET_MATHFILE_SPEC " " \
>> + ? ? ? ? ? ? ? ? ? ?ANDROID_ENDFILE_SPEC)
>
>
> --
> Maxim Kuvyrkov
> CodeSourcery / Mentor Graphics
>
>


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