Re: [PATCH v2] RISC-V: Gate opcode tables by enum rather than string.

[Jim Wilson <jimw at sifive dot com>]

On Mon, Aug 26, 2019 at 3:58 AM Maxim Blinov <maxim.blinov@embecosm.com> wrote:
> include/ChangeLog:
>         * opcode/riscv.h (riscv_insn_class): New enum.
>         * opcode/riscv.h (struct riscv_opcode): Change
>         "subset" field to "insn_class" field.
> gas/ChangeLog:
>         * config/tc-riscv.c (riscv_multi_subset_supports):
>         Handle "insn_class" enum rather than "subset" char string.
> opcodes/ChangeLog:
>         * opcodes/riscv-opc.c: Change "susbet" field to "insn_class" field
>         for all instructions.

The idea looks OK.

The naming of the instruction classes looks a little confusing.
INSN_CLASS_F_C is F and C.  But INSN_CLASS_B_ZBB is B or ZBB.  It
isn't obvious from looking at the names that they are different
operations.  You also have INSN_CLASS_B_ZBT which is ZBT only.
Shouldn't that be named just INSN_CLASS_ZBT?

You have included B extension support, but there is no B extension
support in the ISA standard as yet.  So far, we've been holding off
support for draft extensions, so that we don't get stuck supporting
unofficial versions of extensions.  The exact set of ZB* names could
change before the B extension gets accepted into the standard.

Otherwise this looks OK to me.

Jim