[PATCH v4 1/3] RISC-V: Support the ISA-dependent CSR checking.

[Nelson Chu]

Hi Andrew,

On Tue, Feb 11, 2020 at 7:32 PM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> The changes in gdb/ are approved.  I'm not a binutils maintainer so
> can't approve anything else, but I did look through and had some
> comments, see inline below.

Thank you very much for the approval in gdb side.

> > +typedef struct
> > +{
> > +  /* Class to which this CSR belongs.  Used to decide whether or
> > +     not this CSR is legal in the current -march context.  */
> > +  enum riscv_csr_class csr_class;
> > +} riscv_csr_extra;
>
> The use of 'struct NAME { ... }' and 'typedef struct { ... } NAME'
> seems to be used inconsistently throughout the assembler, but in this
> file it seems like only 'struct NAME { ... }' is used.  It might be
> nice to remain consistent.

Got it, remain the consistent usage is good to me.

> > +/* Init two hashes, csr_extra_hash and reg_names_hash, for CSR.  */
> > +
> > +static void
> > +riscv_init_csr_hashes (const char *name,
> > +                    unsigned address,
> > +                    enum riscv_csr_class class)
> > +{
> > +  riscv_csr_extra *entry = XNEW (riscv_csr_extra);
> > +  entry->csr_class = class;
> > +
> > +  const char *hash_error =
> > +    hash_insert (csr_extra_hash, name, (void *) entry);
> > +  if (hash_error)
>
> I don't know if the assembler has a coding standard, but my preference
> is always 'if (hash_error != NULL)' as hash_error is not a boolean.

This is good to me.  I find the similar usage in init_opcode_hash, so
I also update it to keep the consistent usage in init_opcode_hash and
riscv_init_csr_hashes.

> Like I said, not a binutils maintainer, so feel free to ignore this
> feedback :)

I appreciate any suggestions.  Your suggestion is good :)  I will
update your suggestion and then sent to you and binutils mail list.
Thanks again for your help.

Nelson