On 04/06/2020 12:25, Vineet Gupta wrote:
> On 6/3/20 12:58 PM, Adhemerval Zanella via Libc-alpha wrote:
>>
>>>
>>> diff --git a/config.h.in b/config.h.in
>>> index dea43df438f6..08962dfed075 100644
>>> --- a/config.h.in
>>> +++ b/config.h.in
>>> @@ -109,6 +109,9 @@
>>> /* AArch64 big endian ABI */
>>> #undef HAVE_AARCH64_BE
>>>
>>> +/* ARC big endian ABI */
>>> +#undef HAVE_ARC_BE
>>> +
>>
>> Why do you need this define exactly? It is not used anywhere in the code
>> and for C code if is more straightforwar to use endian.h.
>
> It is used in build system as part of "formal" BE ABI support as pointed to in v4
> series review. This specific thing helps choose the right dynamic linker name for
> BE case.
Ack.
>
> $ git grep HAVE_ARC_BE
> config.h.in:113:#undef HAVE_ARC_BE
> sysdeps/arc/configure:175: $as_echo "#define HAVE_ARC_BE 1" >>confdefs.h
> sysdeps/arc/configure.ac:22: AC_DEFINE(HAVE_ARC_BE)
> sysdeps/unix/sysv/linux/arc/shlib-versions:3:%ifdef HAVE_ARC_BE
>
> I looked at other ports and they seem to follow similar patters: csky for CSKYABI,
> riscv for RISCV_ABI_XLEN etc.
Right, it is the usual way indeed. This is fine.
>
> But I can rework if there's a simpler/better way.
>
>>> +++ b/sysdeps/arc/Versions
>>> @@ -0,0 +1,8 @@
>>> +libc {
>>> + GLIBC_2.32 {
>>> + __mcount;
>>> + }
>>
>> Hum, does ARC require a different symbol name than the one provided
>> by gmon/Versions?
>
> ARC gcc generates __mcount and _mcount with different ABIs and we use the newer
> __mcount.
Ack.
>
>>> +AC_DEFINE(PI_STATIC_AND_HIDDEN)
>>> +libc_cv_have_sdata_section=no
>>
>> The libc_cv_have_sdata_section is done by configure.ac, why do you need
>> to explicit set it here?
>
> We inhibit small data explicitly which by default kicks in.
Ok, is it some limitation for loader bootstrap or something else?
>
>>> +if test $libc_cv_arc_be = yes; then
>>> + # For shlib-versions.
>>> + AC_DEFINE(HAVE_ARC_BE)
>>> + LIBC_CONFIG_VAR([default-abi], [ilp32_be])
>>> +else
>>> + LIBC_CONFIG_VAR([default-abi], [ilp32])
>>> +fi
>>
>> The ilp32 naming is usually set for ILP32 architectures that uses
>> 64-bit registers type on 32 bit VMA (for instance mips64n32, x32,
>> or aarch64_ilp32). I don't think this is the case for ARC, so I think
>> this naming might be confusing.
>>> +abi-variants := ilp32 ilp32_be
>
> arcle arcbe ?
LGTM.
>
>>> +
>>> +ifeq (,$(filter $(default-abi),$(abi-variants)))
>>> +$(error Unknown ABI $(default-abi), must be one of $(abi-variants))
>>> +endif
>>> +
>>> +abi-ilp32-condition := !defined __BIG_ENDIAN__
>>> +abi-ilp32_be-condition := defined __BIG_ENDIAN__
>>
>> Ok with the 'ilp32' naming module described above.
>
>
>>> diff --git a/sysdeps/unix/sysv/linux/arc/Versions b/sysdeps/unix/sysv/linux/arc/Versions
>>> new file mode 100644
>>> index 000000000000..292f1974b02a
>>> --- /dev/null
>>> +++ b/sysdeps/unix/sysv/linux/arc/Versions
>>> @@ -0,0 +1,16 @@
>>> +ld {
>>> + GLIBC_PRIVATE {
>>> + # used for loading by static libraries
>>> + _dl_var_init;
>>> + }
>>> +}
>>> +libc {
>>> + GLIBC_2.32 {
>>> + _flush_cache;
>>> + cacheflush;
>>> + }
>>> + GLIBC_PRIVATE {
>>> + # A copy of sigaction lives in libpthread, and needs these.
>>> + __default_rt_sa_restorer;
>>> + }
>>> +}
>>
>> Afaik all other ABIs that requires the sa_restores uses a local symbol in
>> libpthread. I don't have a strong preference here.
>
> Do you mean add following to sysdeps/unix/sysv/linux/arc/Makefile
>
> ifeq ($(subdir),nptl)
> libpthread-routines += sigrestorer
> libpthread-shared-only-routines += sigrestorer
> endif
Yeap.
>
> And that is to optimize the reference to restorer as a direct PC relative access
> vs a got reference ?
My understanding is this specific optimization does not really matter:
sigaction is hardly a hotspot and the symbol will be issue by the
kernel itself. AFAIU is more a way to optimize the exported symbols
and simplify the GLIBC_PRIVATE namespace (since the sa_restore usually
has small text size).
>
> It seems even in libc, this is currently not optimal. It seems we need
> libc_hidden_* on restorer.
>
> 0002b020 <__GI___libc_sigaction>:
> 2b020: std.aw r14r15,[sp,-8]
> 2b024: push_s r13
> 2b026: sub_s sp,sp,0x28
> ...
> 2b02e: mov_s r3,r1
> 2b030: ld r13,[pcl,0xd4e9c] <__default_rt_sa_restorer@@GLIBC_PRIVATE+0xd4558>
>
If you define it as attribute_hidden and add on both libc and libpthread it
should not require the libc_hidden_{proto,def}.
>
>
>>> diff --git a/sysdeps/unix/sysv/linux/arc/shlib-versions b/sysdeps/unix/sysv/linux/arc/shlib-versions
>>> new file mode 100644
>>> index 000000000000..343c0a04500e
>>> --- /dev/null
>>> +++ b/sysdeps/unix/sysv/linux/arc/shlib-versions
>>> @@ -0,0 +1,7 @@
>>> +DEFAULT GLIBC_2.32
>>> +
>>> +%ifdef HAVE_ARC_BE
>
> This is where the BE define is used.
>
>>> +ld=ld-linux-arceb.so.2
>>> +%else
>>> +ld=ld-linux-arc.so.2
>>> +%endif
>
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
