Re: [PATCH 13/17] RISC-V: Linux ABI

[Joseph Myers <joseph at codesourcery dot com>]

On Wed, 24 Jan 2018, Palmer Dabbelt wrote:

> diff --git a/sysdeps/unix/sysv/linux/riscv/flush-icache.c b/sysdeps/unix/sysv/linux/riscv/flush-icache.c

> +static int __riscv_flush_icache_syscall (void *start, void *end,
> +					 unsigned long int flags)
> +{
> +	return INLINE_SYSCALL (riscv_flush_icache, 3, start, end, flags);

The formatting is off here.  "static int" on its own line, function name 
at the start of the next line.  return statement should be indented by two 
spaces, not a TAB.

> +  /* RISC-V Linux ABIs mandate the presence of the C extension.  */
> +  if (flags & EF_RISCV_RVC)
> +    return 1;

My understanding is that you should just remove this check; shared 
libraries are OK both with and without EF_RISCV_RVC, so there should be no 
need to reject those with it set (or if you need to reject them, you 
definitely need a different comment explaining why).

> +#ifdef __USE_MISC
> +# define NGREG	32
> +
> +#define REG_PC 0
> +#define REG_RA 1
> +#define REG_SP 2
> +#define REG_TP 4
> +#define REG_S0 8
> +#define REG_S1 9
> +#define REG_A0 10
> +#define REG_S2 18
> +#define REG_NARGS 8

Missing preprocessor indentation, "# define", on all these REG_* macros.

> diff --git a/sysdeps/unix/sysv/linux/riscv/sysdep-cancel.h b/sysdeps/unix/sysv/linux/riscv/sysdep-cancel.h

You shouldn't need this file.  All the architecture-specific 
sysdep-cancel.h files were removed by Adhemerval a while back.

-- 
Joseph S. Myers
joseph@codesourcery.com