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: Joseph Myers <joseph at codesourcery dot com>
- To: Palmer Dabbelt <palmer at dabbelt dot com>
- Cc: <libc-alpha at sourceware dot org>, <patches at groups dot riscv dot org>
- Date: Mon, 1 Jan 2018 13:09:06 +0000
- 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>
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).
> 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.
> +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).
> 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.
--
Joseph S. Myers
joseph@codesourcery.com