Re: [PATCH v6 11/13] ARC: Build Infrastructure

[Vineet Gupta <Vineet.Gupta1@synopsys.com>]

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.

$ 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.

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.

>> +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.

>> +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 ?

>> +
>> +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

And that is to optimize the reference to restorer as a direct PC relative access
vs a got reference ?

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>



>> 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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc