This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v3 07/19] RISC-V: Build Infastructure


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]