Re: [PATCH v3 08/19] RISC-V: ABI Implementation

[Joseph Myers <joseph at codesourcery dot com>]

On Tue, 26 Dec 2017, Palmer Dabbelt wrote:

> diff --git a/sysdeps/riscv/bits/setjmp.h b/sysdeps/riscv/bits/setjmp.h

> +    /* Callee-saved floating point registers.  */
> +#ifdef __riscv_float_abi_single
> +   float __fpregs[12];

More stray "f" ABI support to remove given that ABI's not being supported.

> diff --git a/sysdeps/riscv/dl-trampoline.S b/sysdeps/riscv/dl-trampoline.S

Your ABI document says that the floating-point ABI passes arguments in 
floating-point registers, and those argument registers are not preserved 
across calls.

This means that (in the hard-float ABI case) _dl_runtime_resolve needs to 
preserve those registers in case the dynamic linker clobbers them.  Even 
if you somehow ensure that no dynamic linker code uses those registers 
(e.g. compile the dynamic linker with -ffixed-* options for them), the 
dynamic linker can call user code (audit modules, interposed malloc, IFUNC 
resolvers - you don't have IFUNC support for RISC-V at present, but the 
other cases still apply).  If such user code, called during symbol 
resolution, were to change those call-clobbered argument registers, that 
would corrupt the floating-point arguments to the function being called.

> diff --git a/sysdeps/riscv/machine-gmon.h b/sysdeps/riscv/machine-gmon.h

> +static void mcount_internal (u_long frompc, u_long selfpc);
> +
> +#define _MCOUNT_DECL(frompc, selfpc) \
> +static inline void mcount_internal (u_long frompc, u_long selfpc)
> +
> +#define MCOUNT								\
> +void _mcount (void *frompc)						\
> +{									\
> +  mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0));	\

Please use "unsigned long int", not the legacy "u_long" name, in new code.  
(Note that the sys/gmon.h header, which this is probably meant to match, 
now uses "unsigned long" after Zack's installed header cleanups.)

> diff --git a/sysdeps/riscv/sys/asm.h b/sysdeps/riscv/sys/asm.h

> +/* For ABI uniformity, reserve 8 bytes for floats, even if double-precision
> +   floating-point is not supported in hardware.  */
> +# if defined __riscv_float_abi_single

More stray "f" ABI support here.

> +/*
> + * LEAF - declare leaf routine
> + */

Please see my previous comments in 
<https://sourceware.org/ml/libc-alpha/2017-12/msg00742.html> regarding 
proper GNU comment formatting, for this and other macros in this file.

-- 
Joseph S. Myers
joseph@codesourcery.com