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.enkov...@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
>
>
