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

[Palmer Dabbelt <palmer at dabbelt dot com>]

On Thu, 25 Jan 2018 09:24:45 PST (-0800), joseph@codesourcery.com wrote:
> 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).

Odd, I remember deleting that check...  I guess I must just be crazy!  It 
should really be gone now.

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

Thanks, I must have missed those when going through last time.

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

OK, it's gone now.

Thanks!