[Integration 1/2] RISC-V: add vendor opcodes
Nelson Chu
nelson.chu@sifive.com
Fri Sep 3 04:00:53 GMT 2021
On Fri, Sep 3, 2021 at 11:00 AM Lifang Xia <lifang_xia@c-sky.com> wrote:
>
> This patch uses the T-HEAD Xuantie processor as an example to solve that
> how to add the vendor's opcode and CSR.
> Without modifying the existing mechanism, create a new
> opcodes/riscv-vendor-opc.c, similar to opcodes/riscv-opc.c, used to
> describe the vendor's opcodes.
> New file, include/opcodes/riscv-vendor-opc.h, used to define the encoding,
> mask and CSR of the opcodes.
>
> The sub-string from -march option is used to pick the suitable list of
> the vendor's opcodes.
>
> bfd/
> * cpu-riscv.h (enum riscv_spec_class): New
> ISA_SPEC_CLASS_VENDOR.
> gas/
> * config/tc-riscv.c (ext_version_table): New "theadc".
> (riscv_vendor_get_opcodes): New.
> (riscv_multi_subset_supports): Check xtheadc extensions.
> (riscv_get_default_ext_version): Skip for vendor.
> (init_opcode_hash): Add vendor's extensions to the hash.
> (md_begin): Init opcode hash.
> include/
> * opcode/riscv-opc.h: Include riscv-vendor-opc.h
> * opcode/riscv-vendor-opc.h: New.
> * opcode/riscv.h (enum riscv_extended_insn_class): New class for vendor.
> (struct riscv_vendor_opcode): New.
> (riscv_vendor_list): New.
> opcodes/
> * Makefile.am: Add riscv-vendor-opc.c.
> * Makefile.in: Likewise.
> * configure: Likewise
> * configure.ac: Likewise
> * riscv-dis.c (arch_string): New.
> (get_vendor_opcodes): Get the vendor opcodes.
> (riscv_disassemble_insn): Add vendor's opcode to the hash.
> (riscv_get_disassembler): Setup arch_string with attr.
> * riscv-vendor-opc.c: New.
> ---
> bfd/cpu-riscv.h | 1 +
> gas/config/tc-riscv.c | 55 +++++++++++++++++++++++++++++--
> include/opcode/riscv-opc.h | 1 +
> include/opcode/riscv-vendor-opc.h | 34 +++++++++++++++++++
> include/opcode/riscv.h | 10 ++++++
> opcodes/Makefile.am | 1 +
> opcodes/Makefile.in | 2 ++
> opcodes/configure | 2 +-
> opcodes/configure.ac | 2 +-
> opcodes/riscv-dis.c | 23 +++++++++++++
> opcodes/riscv-vendor-opc.c | 43 ++++++++++++++++++++++++
> 11 files changed, 170 insertions(+), 4 deletions(-)
> create mode 100644 include/opcode/riscv-vendor-opc.h
> create mode 100644 opcodes/riscv-vendor-opc.c
>
> diff --git a/bfd/cpu-riscv.h b/bfd/cpu-riscv.h
> index cafaca23be0..06ede1b2bef 100644
> --- a/bfd/cpu-riscv.h
> +++ b/bfd/cpu-riscv.h
> @@ -26,6 +26,7 @@ enum riscv_spec_class
> ISA_SPEC_CLASS_20190608,
> ISA_SPEC_CLASS_20191213,
> ISA_SPEC_CLASS_DRAFT,
> + ISA_SPEC_CLASS_VENDOR,
The basic idea of riscv_spec_class is that - Every spec should have
their own names and versions, so the ISA_SPEC is riscv ISA spec,
PRIV_SPEC is the privileged spec, we also may have DEBUG_SPEC for
debug. Therefore, each vendor should have it's own <VENDOR_NAME>_SPEC
and versions. For example, maybe SIFIVE_XXX_SPEC_1p0, .... The riscv
isa spec doesn't define vendor stuff, so ISA_SPEC_CLASS_VENDOR isn't
suitable for me.
> /* Privileged spec. */
> PRIV_SPEC_CLASS_NONE,
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 18992949954..99bc9298b16 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -157,6 +157,9 @@ static const struct riscv_ext_version ext_version_table[] =
> {"zba", ISA_SPEC_CLASS_DRAFT, 0, 93},
> {"zbc", ISA_SPEC_CLASS_DRAFT, 0, 93},
>
> + /* T-HEAD extentions for Xuantie C9xx. */
> + {"xtheadc", ISA_SPEC_CLASS_VENDOR, 2, 0},
Likewise.
> /* Terminate the list. */
> {NULL, 0, 0, 0}
> };
> @@ -238,6 +241,22 @@ riscv_set_default_isa_spec (const char *s)
> return 1;
> }
>
> +/* Get the opcodes from vendors. */
> +static struct riscv_opcode *
> +riscv_vendor_get_opcodes (riscv_subset_list_t *subsets)
> +{
> + unsigned int i = 0;
> + struct riscv_subset_t *subset = NULL;
> +
> + for (i = 0; riscv_vendor_list[i].vendor; i++)
> + {
> + if (riscv_lookup_subset (subsets, riscv_vendor_list[i].vendor, &subset))
> + return riscv_vendor_list[i].opcodes;
> + }
> +
> + return NULL;
> +}
Each function should have the corresponding extended function, which
is used to handle draft or vendor stuff. Could you port your vendor
extensions into these extended functions? For your
riscv_vendor_get_opcodes, you just need to add your vendor opcodes
into riscv_extended_opcodes, and then update the
riscv_find_extended_opcode_hash. You don't need to build the whole
vendor structures again, I suppose the extended functions should be
enough to handle most of the vendor extensions, except the vendor
relocations.
> /* Set the default_priv_spec. Find the privileged elf attributes when
> the input string is NULL. Return 0 if the spec isn't supported.
> Otherwise, return 1. */
> @@ -407,6 +426,9 @@ riscv_multi_subset_supports (enum riscv_insn_class insn_class)
> case INSN_CLASS_ZBC:
> return riscv_subset_supports ("zbc");
>
> + case INSN_CLASS_THEADC:
> + return riscv_subset_supports ("xtheadc");
Likewise, it is better to be added in the riscv_extended_subset_supports.
> default:
> return riscv_extended_subset_supports (insn_class);
> }
> @@ -458,6 +480,7 @@ riscv_search_ext_version_hash (const char *name,
> && strcmp (ext->name, name) == 0)
> {
> if (ext->isa_spec_class == ISA_SPEC_CLASS_DRAFT
> + || ext->isa_spec_class == ISA_SPEC_CLASS_VENDOR
> || ext->isa_spec_class == default_isa_spec)
> {
> *major_version = ext->major_version;
> @@ -1328,6 +1351,7 @@ struct percent_op_match
>
> static htab_t
> init_opcode_hash (const struct riscv_opcode *opcodes,
> + const struct riscv_opcode *ext_opcodes,
> bool insn_directive_p)
> {
> int i = 0;
> @@ -1358,6 +1382,29 @@ init_opcode_hash (const struct riscv_opcode *opcodes,
> while (opcodes[i].name && !strcmp (opcodes[i].name, name));
> }
>
> + i = 0;
> + while (ext_opcodes && ext_opcodes[i].name)
> + {
> + const char *name = ext_opcodes[i].name;
> + if (str_hash_insert (hash, name, &ext_opcodes[i], 0) != NULL)
> + as_fatal (_("internal: duplicate %s"), name);
> +
> + do
> + {
> + if (ext_opcodes[i].pinfo != INSN_MACRO)
> + {
> + length = 0; /* Let assembler determine the length. */
> + if (!validate_riscv_insn (&ext_opcodes[i], length))
> + as_fatal (_("internal: broken assembler. "
> + "No assembly attempted"));
> + }
> + else
> + gas_assert (!insn_directive_p);
> + ++i;
> + }
> + while (ext_opcodes[i].name && !strcmp (ext_opcodes[i].name, name));
> + }
> +
> return hash;
> }
Please check the md_begin, and see how draft extensions are
initialized. I think you can create a new vendor opcode hash, and
just call the init_opcode_hash.
> @@ -1372,8 +1419,11 @@ md_begin (void)
> if (! bfd_set_arch_mach (stdoutput, bfd_arch_riscv, mach))
> as_warn (_("could not set architecture and machine"));
>
> - op_hash = init_opcode_hash (riscv_opcodes, false);
> - insn_type_hash = init_opcode_hash (riscv_insn_types, true);
> + op_hash = init_opcode_hash (riscv_opcodes,
> + riscv_vendor_get_opcodes (&riscv_subsets),
> + false);
> +
> + insn_type_hash = init_opcode_hash (riscv_insn_types, NULL, true);
Likewise, don't need to change this.
> reg_names_hash = str_htab_create ();
> hash_reg_names (RCLASS_GPR, riscv_gpr_names_numeric, NGPR);
> @@ -4587,3 +4637,4 @@ riscv_pop_insert (void)
>
> pop_insert (riscv_pseudo_table);
> }
> +
> diff --git a/include/opcode/riscv-opc.h b/include/opcode/riscv-opc.h
> index 9999da6241a..84fc2ee0e8e 100644
> --- a/include/opcode/riscv-opc.h
> +++ b/include/opcode/riscv-opc.h
> @@ -20,6 +20,7 @@
>
> #ifndef RISCV_ENCODING_H
> #define RISCV_ENCODING_H
> +#include "opcode/riscv-vendor-opc.h"
> /* Instruction opcode macros. */
> #define MATCH_SLLI_RV32 0x1013
> #define MASK_SLLI_RV32 0xfe00707f
> diff --git a/include/opcode/riscv-vendor-opc.h b/include/opcode/riscv-vendor-opc.h
Could these encodings be added in the riscv-opc-extended.h?
> new file mode 100644
> index 00000000000..949047239f8
> --- /dev/null
> +++ b/include/opcode/riscv-vendor-opc.h
> @@ -0,0 +1,34 @@
> +/* riscv-vendor-opc.h. RISC-V instruction opcode and CSR macros from vendors.
> + Copyright (C) 2011-2021 Free Software Foundation, Inc.
> + Contributed by Andrew Waterman
> +
> + This file is part of GDB, GAS, and the GNU binutils.
> +
> + GDB, GAS, and the GNU binutils are free software; you can redistribute
> + them and/or modify them under the terms of the GNU General Public
> + License as published by the Free Software Foundation; either version
> + 3, or (at your option) any later version.
> +
> + GDB, GAS, and the GNU binutils are distributed in the hope that they
> + will be useful, but WITHOUT ANY WARRANTY; without even the implied
> + warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> + the GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program; see the file COPYING3. If not,
> + see <http://www.gnu.org/licenses/>. */
> +
> +#ifndef __RISCV_VENDOR_OPC_H__
> +#define __RISCV_VENDOR_OPC_H__
> +
> +
> +/* Opcodes for VENDOR 0. */
> +
> +
> +/* Opcodes for VENDOR 1. */
> +
> +
> +/* Opcodes for VENDOR 2. */
> +
> +
> +#endif
> diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> index 8d5251c50e4..af8df794d1b 100644
> --- a/include/opcode/riscv.h
> +++ b/include/opcode/riscv.h
I think just updating the extended structures and classes should be enough.
> @@ -361,6 +361,12 @@ struct riscv_opcode
> unsigned long pinfo;
> };
>
> +struct riscv_vendor_opcode
> +{
> + const char *vendor;
> + struct riscv_opcode *opcodes;
> +};
> +
> /* Instruction is a simple alias (e.g. "mv" for "addi"). */
> #define INSN_ALIAS 0x00000001
>
> @@ -434,6 +440,7 @@ extern const char * const riscv_fpr_names_abi[NFPR];
>
> extern const struct riscv_opcode riscv_opcodes[];
> extern const struct riscv_opcode riscv_insn_types[];
> +extern const struct riscv_vendor_opcode riscv_vendor_list[];
You don't need to add this, just add your vendor opcodes into
riscv_extended_opcodes in the opcodes/riscv-opc.c.
> /* Extended extensions. */
>
> @@ -496,6 +503,9 @@ enum riscv_extended_insn_class
> INSN_CLASS_D_AND_ZFH,
> INSN_CLASS_Q_AND_ZFH,
> INSN_CLASS_SVINVAL,
> +
> + /* INSN class for THEAD. */
> + INSN_CLASS_THEADC,
> };
Yes, this one is good, added vendor stuff into extended classes is
what I suggest.
> /* This is a list of macro expanded instructions for extended
> diff --git a/opcodes/Makefile.am b/opcodes/Makefile.am
> index 0e04b4c05c4..4f3ad0e69cd 100644
> --- a/opcodes/Makefile.am
> +++ b/opcodes/Makefile.am
> @@ -229,6 +229,7 @@ TARGET_LIBOPCODES_CFILES = \
> pru-opc.c \
> riscv-dis.c \
> riscv-opc.c \
> + riscv-vendor-opc.c \
> rl78-decode.c \
> rl78-dis.c \
> rx-decode.c \
> diff --git a/opcodes/Makefile.in b/opcodes/Makefile.in
> index 42c15f00d30..5dcbc204a1e 100644
> --- a/opcodes/Makefile.in
> +++ b/opcodes/Makefile.in
> @@ -620,6 +620,7 @@ TARGET_LIBOPCODES_CFILES = \
> pru-opc.c \
> riscv-dis.c \
> riscv-opc.c \
> + riscv-vendor-opc.c \
> rl78-decode.c \
> rl78-dis.c \
> rx-decode.c \
> @@ -1036,6 +1037,7 @@ distclean-compile:
> @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/pru-opc.Plo@am__quote@
> @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/riscv-dis.Plo@am__quote@
> @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/riscv-opc.Plo@am__quote@
> +@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/riscv-vendor-opc.Plo@am__quote@
> @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/rl78-decode.Plo@am__quote@
> @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/rl78-dis.Plo@am__quote@
> @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/rx-decode.Plo@am__quote@
> diff --git a/opcodes/configure b/opcodes/configure
> index 3513e408ce1..ad9ec966411 100755
> --- a/opcodes/configure
> +++ b/opcodes/configure
> @@ -12265,7 +12265,7 @@ if test x${all_targets} = xfalse ; then
> bfd_pru_arch) ta="$ta pru-dis.lo pru-opc.lo" ;;
> bfd_pyramid_arch) ;;
> bfd_romp_arch) ;;
> - bfd_riscv_arch) ta="$ta riscv-dis.lo riscv-opc.lo" ;;
> + bfd_riscv_arch) ta="$ta riscv-dis.lo riscv-opc.lo riscv-vendor-opc.lo" ;;
> bfd_rs6000_arch) ta="$ta ppc-dis.lo ppc-opc.lo" ;;
> bfd_rl78_arch) ta="$ta rl78-dis.lo rl78-decode.lo";;
> bfd_rx_arch) ta="$ta rx-dis.lo rx-decode.lo";;
> diff --git a/opcodes/configure.ac b/opcodes/configure.ac
> index e564f067334..16024f2f0d5 100644
> --- a/opcodes/configure.ac
> +++ b/opcodes/configure.ac
> @@ -326,7 +326,7 @@ if test x${all_targets} = xfalse ; then
> bfd_pru_arch) ta="$ta pru-dis.lo pru-opc.lo" ;;
> bfd_pyramid_arch) ;;
> bfd_romp_arch) ;;
> - bfd_riscv_arch) ta="$ta riscv-dis.lo riscv-opc.lo" ;;
> + bfd_riscv_arch) ta="$ta riscv-dis.lo riscv-opc.lo riscv-vendor-opc.lo" ;;
> bfd_rs6000_arch) ta="$ta ppc-dis.lo ppc-opc.lo" ;;
> bfd_rl78_arch) ta="$ta rl78-dis.lo rl78-decode.lo";;
> bfd_rx_arch) ta="$ta rx-dis.lo rx-decode.lo";;
We all have a consensus that no longer need to add new files for draft
and vendor stuff. I cannot find the first mailing discussions about
my first integration patches, but you can see this one:
https://sourceware.org/pipermail/binutils/2021-March/115990.html.
Anyway, I suppose all you need to do is to port vendor extensions into
the existing extended functions.
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index dcc5a09213b..8ddc6bec370 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -33,6 +33,7 @@
> #include <ctype.h>
>
> static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_NONE;
> +static const char *arch_string = NULL;
>
> struct riscv_private_data
> {
> @@ -140,6 +141,21 @@ parse_riscv_dis_options (const char *opts_in)
> free (opts);
> }
>
> +static struct riscv_opcode*
> +get_vendor_opcodes (const char *arch)
> +{
> + int i = 0;
> + if (arch == NULL)
> + return NULL;
> +
> + for (i = 0; riscv_vendor_list[i].vendor; i++)
> + {
> + if (strstr(arch, riscv_vendor_list[i].vendor) != NULL)
> + return riscv_vendor_list[i].opcodes;
> + }
> + return NULL;
> +}
> +
> /* Print one argument from an array. */
>
> static void
> @@ -548,6 +564,11 @@ riscv_disassemble_opcode (insn_t word,
> if (!riscv_hash[OP_HASH_IDX (op->match)])
> riscv_hash[OP_HASH_IDX (op->match)] = op;
>
> + op = get_vendor_opcodes (arch_string);
> + for (; op && op->name; op++)
> + if (!riscv_hash[OP_HASH_IDX (op->match)])
> + riscv_hash[OP_HASH_IDX (op->match)] = op;
> +
> init = 1;
> }
>
> @@ -744,6 +765,8 @@ riscv_get_disassembler (bfd *abfd)
> attr[Tag_b].i,
> attr[Tag_c].i,
> &default_priv_spec);
> + /* Get Vendor opcodes. */
> + arch_string = attr[Tag_RISCV_arch].s;
> }
> }
> }
Likewise, if you add your vendor opcode into the
riscv_extended_opcodes, then you don't need to do anything here,
except that you have vendor operands. Then you will need to update
your vendor operands in print_extended_insn_args, that's all.
> diff --git a/opcodes/riscv-vendor-opc.c b/opcodes/riscv-vendor-opc.c
> new file mode 100644
> index 00000000000..9191b654c14
> --- /dev/null
> +++ b/opcodes/riscv-vendor-opc.c
> @@ -0,0 +1,43 @@
> +/* RISC-V opcode list from vendors.
> + Copyright (C) 2011-2021 Free Software Foundation, Inc.
> +
> + Contributed by Andrew Waterman (andrew@sifive.com).
> + Based on MIPS target.
> +
> + This file is part of the GNU opcodes library.
> +
> + This library is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3, or (at your option)
> + any later version.
> +
> + It is distributed in the hope that it will be useful, but WITHOUT
> + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
> + License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program; see the file COPYING3. If not,
> + see <http://www.gnu.org/licenses/>. */
> +
> +
> +#include "sysdep.h"
> +#include "opcode/riscv.h"
> +#include <stdio.h>
> +#include "bfd.h"
> +
> +/* The vendor extension opcodes for T-HEAD. */
> +struct riscv_opcode vendor_thead_opcodes[] =
> +{
> + {0, 0, INSN_CLASS_NONE, 0, 0, 0, 0, 0}
> +
> +};
> +
> +/* The vendor opcodes list. */
> +const struct riscv_vendor_opcode riscv_vendor_list[] =
> +{
> + /* Vendor T-HEAD. */
> + {"xtheadc", vendor_thead_opcodes},
> + {NULL, NULL},
> +};
> +
> --
> 2.17.1
>
More information about the Binutils
mailing list