This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v3 07/19] RISC-V: Build Infastructure
- From: Darius Rad <darius at bluespec dot com>
- To: Joseph Myers <joseph at codesourcery dot com>, Palmer Dabbelt <palmer at dabbelt dot com>
- Cc: libc-alpha at sourceware dot org, patches at groups dot riscv dot org
- Date: Thu, 4 Jan 2018 22:24:48 -0500
- Subject: Re: [PATCH v3 07/19] RISC-V: Build Infastructure
- Authentication-results: sourceware.org; auth=none
- References: <20171227060534.3998-1-palmer@dabbelt.com> <20171227060534.3998-8-palmer@dabbelt.com> <alpine.DEB.2.20.1801011302230.23191@digraph.polyomino.org.uk>
On 01/01/2018 08:09 AM, Joseph Myers wrote:
> On Tue, 26 Dec 2017, Palmer Dabbelt wrote:
>
>> diff --git a/sysdeps/riscv/Versions b/sysdeps/riscv/Versions
>> new file mode 100644
>> index 000000000000..aafa348a2395
>> --- /dev/null
>> +++ b/sysdeps/riscv/Versions
>> @@ -0,0 +1,4 @@
>> +libc {
>> + GLIBC_2.27 {
>> + }
>> +}
>
> I still don't see why this file should be needed. You need more
> explanation than "IIRC some test was depending on this." for such an empty
> GLIBC_2.27 version here (and the explanation ought to go in a comment in
> the file to justify why the file exists).
>
It doesn't appear to be necessary (I just tried without it), so I'll
remove it. Thanks.
>> diff --git a/sysdeps/riscv/preconfigure b/sysdeps/riscv/preconfigure
>> + case "$float_abi" in
>> + soft)
>> + abi_flen=0
>> + ;;
>> + single)
>> + abi_flen=32
>> + ;;
>
> I'd expect the "single" case to be removed or made into an error, given
> the decision not to support the "f" ABIs.
>
Indeed. I'll remove references to the single precision ABIs here and
elsewhere.
>> +abi-ilp32-condition := (__SIZEOF_INT__ == 4)
>> +abi-ilp32-condition += && (__SIZEOF_LONG__ == 4)
>> +abi-ilp32-condition += && (__SIZEOF_POINTER__ == 4)
>> +abi-ilp32-condition += && (defined __riscv_float_abi_soft)
>> +abi-ilp32-condition += && (!defined __riscv_float_abi_single)
>> +abi-ilp32-condition += && (!defined __riscv_float_abi_double)
>
> You don't need such long and redundant conditions. I'd expect something
> simpler like on other architectures (e.g. !defined __LP64__ && defined
> __riscv_float_abi_soft in this case - in general, just checking one macro
> for the ILP32/LP64 distinction, one for the floating-point ABI).
>
I'll remove the extraneous checks for the floating point ABI. However,
I think the other checks are necessary, as __LP64__/__ILP32__ are not
defined by gcc when -mabi is used. That might be a gcc bug, but that
seems to be how it is now. Alternatively, I could replace the check
with __riscv_xlen == 32 and == 64, but that doesn't seem like a good
approach, as it would break, if, for example, RISC-V ever supported
ILP32 on RV64.
>> diff --git a/sysdeps/unix/sysv/linux/riscv/configure.ac b/sysdeps/unix/sysv/linux/riscv/configure.ac
>
>> +AC_EGREP_CPP(yes, [#ifdef __riscv_float_abi_single
>> + yes
>> + #endif
>> + ],libc_cv_riscv_float_abi=f)
>
> I'd expect this "f" ABI support to go away.
>
>> diff --git a/sysdeps/unix/sysv/linux/riscv/shlib-versions b/sysdeps/unix/sysv/linux/riscv/shlib-versions
>
>> +%elif RISCV_ABI_XLEN == 64 && RISCV_ABI_FLEN == 32
>> +ld=ld-linux-riscv64-lp64f.so.1
>
>> +%elif RISCV_ABI_XLEN == 32 && RISCV_ABI_FLEN == 32
>> +ld=ld-linux-riscv32-ilp32f.so.1
>
> Again, I'd expect the "f" ABI support to go away.
>