[RFC 1/4] RISC-V: Hypervisor ext: Treat as "Standard" extension
Nelson Chu
nelson.chu@sifive.com
Fri Dec 17 16:10:42 GMT 2021
On Fri, Dec 17, 2021 at 12:10 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 16 Dec 2021 09:33:25 PST (-0800), Vineet Gupta wrote:
> > gas currently fails for single-letter 'h' prefix in arch string.
> >
> > | riscv64-unknown-elf-as -march=rv64gh empty.s
> > | Assembler messages:
> > | Error: rv64gh: unknown prefixed ISA extension `h'
> >
> > It implements the "multi-letter" prefix for H-ext as recommended in
> > ISA manual subsection "ISA Extension Naming Conventions". However given
> > the discussions at [1] there seems to be no pressing need to keep version
> > for the current H-ext since
> > - it will remain at 1.0
> > - any future Hypervisor extensions wull called "Sh*" or some such
> > - H was the original standard extension with its own MISA bit
> >
> > This patch thus relaxes the constraint and allows single letter prefix
> > 'h'
> >
> > Note: To keep the binutils dejagnu failures same, I've fixed the
> > testsuite files within the same patch. I can break it up if reviewers
> > feel as such.
> >
> > [1] https://github.com/riscv/riscv-isa-manual/issues/781#issuecomment-983233088
> >
> > bfd/
> > * elfxx-riscv.c (riscv_supported_std_ext): Add "h" entry.
> > (riscv_supported_std_h_ext): Add "h" entry.
> > (riscv_get_default_ext_version): Handle PRIV_SPEC_CLASS_DRAFT.
> > (riscv_multi_subset_supports): Handle INSN_CLASS_H.
> >
> > gas/
> > * testsuite/gas/riscv/march-fail-single-prefix-h.d: Deleted as
> > single-letter prefix 'h' is now valid.
> > * testsuite/gas/riscv/march-fail-single-prefix-l: Removed 'h'.
> >
> > include/
> > * opcode/riscv.h (enum riscv_insn_class): Add INSN_CLASS_H.
> >
> > Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
> > ---
> > bfd/elfxx-riscv.c | 5 +++++
> > gas/testsuite/gas/riscv/march-fail-single-prefix-h.d | 3 ---
> > gas/testsuite/gas/riscv/march-fail-single-prefix.l | 2 +-
> > include/opcode/riscv.h | 1 +
> > 4 files changed, 7 insertions(+), 4 deletions(-)
> > delete mode 100644 gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
> >
> > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> > index 3bd41ff2b551..7d5ae5a4657d 100644
> > --- a/bfd/elfxx-riscv.c
> > +++ b/bfd/elfxx-riscv.c
> > @@ -1173,6 +1173,7 @@ static struct riscv_supported_ext riscv_supported_std_ext[] =
> > {"t", ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
> > {"p", ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
> > {"v", ISA_SPEC_CLASS_DRAFT, 1, 0, 0 },
> > + {"h", PRIV_SPEC_CLASS_DRAFT, 1, 0, 0 },
>
> According to
> <https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions>, this
> was ratified in November so we should be able to update this to
> something like PRIV_SPEC_CLASS_1P12 (the V stuff was also ratified, but
> that's a different issue).
I can understand that why Vineet are doing these changes, but I had a
quick talk with Kito this morning, and I think we also have three
issues for now,
1. The current draft ISA spec doesn't have any related changes, so if
we change the `h' as a single standard extension, then this will
conflict with the spec.
2. Even if we decide to change the `h' as a single extension rather
than the multiple prefixed keyword, then in what order should we place
h. I expect the table 27.1 in the ISA spec will also mention the
order of the single h.
3. I never considered that an extension version may be controlled by
the privileged spec before, so in the riscv_get_default_ext_version,
> > if (strcmp (table[i].name, name) == 0
> > && (table[i].isa_spec_class == ISA_SPEC_CLASS_DRAFT
> > + || table[i].isa_spec_class == PRIV_SPEC_CLASS_DRAFT
> > || table[i].isa_spec_class == *default_isa_spec))
We will need to rewrite the related code, since the isa_spec_class may
be one of the PRIV_SPEC_CLASS_XXX in the future, so we will never
match it to the default_isa_spec...
> > {"n", ISA_SPEC_CLASS_NONE, RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION, 0 },
> > {NULL, 0, 0, 0, 0}
> > };
> > @@ -1232,6 +1233,7 @@ static struct riscv_supported_ext riscv_supported_std_s_ext[] =
> >
> > static struct riscv_supported_ext riscv_supported_std_h_ext[] =
> > {
> > + {"h", PRIV_SPEC_CLASS_DRAFT, 1, 0, 0 },
> > {NULL, 0, 0, 0, 0}
> > };
> >
> > @@ -1531,6 +1533,7 @@ riscv_get_default_ext_version (enum riscv_spec_class *default_isa_spec,
> > {
> > if (strcmp (table[i].name, name) == 0
> > && (table[i].isa_spec_class == ISA_SPEC_CLASS_DRAFT
> > + || table[i].isa_spec_class == PRIV_SPEC_CLASS_DRAFT
> > || table[i].isa_spec_class == *default_isa_spec))
> > {
> > *major_version = table[i].major_version;
> > @@ -2412,6 +2415,8 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
> > || riscv_subset_supports (rps, "zve64d")
> > || riscv_subset_supports (rps, "zve64f")
> > || riscv_subset_supports (rps, "zve32f"));
> > + case INSN_CLASS_H:
> > + return riscv_subset_supports (rps, "h");
> > default:
> > rps->error_handler
> > (_("internal: unreachable INSN_CLASS_*"));
> > diff --git a/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d b/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
> > deleted file mode 100644
> > index eb101bd71353..000000000000
> > --- a/gas/testsuite/gas/riscv/march-fail-single-prefix-h.d
> > +++ /dev/null
> > @@ -1,3 +0,0 @@
> > -#as: -march=rv32ih
> > -#source: empty.s
> > -#error_output: march-fail-single-prefix.l
> > diff --git a/gas/testsuite/gas/riscv/march-fail-single-prefix.l b/gas/testsuite/gas/riscv/march-fail-single-prefix.l
> > index 13942ed66642..e4418de69acb 100644
> > --- a/gas/testsuite/gas/riscv/march-fail-single-prefix.l
> > +++ b/gas/testsuite/gas/riscv/march-fail-single-prefix.l
> > @@ -1,2 +1,2 @@
> > .*Assembler messages:
> > -.*Error: .*unknown prefixed ISA extension `(s|h|z|x|zxm)'
> > +.*Error: .*unknown prefixed ISA extension `(s|z|x|zxm)'
> > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> > index 14889972abce..2783bdecaae5 100644
> > --- a/include/opcode/riscv.h
> > +++ b/include/opcode/riscv.h
> > @@ -387,6 +387,7 @@ enum riscv_insn_class
> > INSN_CLASS_ZKND_OR_ZKNE,
> > INSN_CLASS_V,
> > INSN_CLASS_ZVEF,
> > + INSN_CLASS_H,
> > };
> >
> > /* This structure holds information for a particular instruction. */
>
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
>
> We can always deal with the draft/ratified stuff later, as there's a
> bunch of them.
More information about the Binutils
mailing list