[PATCH v5 0/4] target-riscv: support vector extension part 1

LIU Zhiwei posted 4 patches 4 years, 1 month ago
Test docker-quick@centos7 passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200221094531.61894-1-zhiwei_liu@c-sky.com
Maintainers: Sagar Karandikar <sagark@eecs.berkeley.edu>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <Alistair.Francis@wdc.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
There is a newer version of this series
MAINTAINERS                             |  1 +
target/riscv/Makefile.objs              |  2 +-
target/riscv/cpu.c                      |  7 +++
target/riscv/cpu.h                      | 78 ++++++++++++++++++++++---
target/riscv/cpu_bits.h                 | 15 +++++
target/riscv/csr.c                      | 75 +++++++++++++++++++++++-
target/riscv/helper.h                   |  2 +
target/riscv/insn32.decode              |  5 ++
target/riscv/insn_trans/trans_rvv.inc.c | 69 ++++++++++++++++++++++
target/riscv/translate.c                | 17 +++++-
target/riscv/vector_helper.c            | 53 +++++++++++++++++
11 files changed, 312 insertions(+), 12 deletions(-)
create mode 100644 target/riscv/insn_trans/trans_rvv.inc.c
create mode 100644 target/riscv/vector_helper.c
[PATCH v5 0/4] target-riscv: support vector extension part 1
Posted by LIU Zhiwei 4 years, 1 month ago
This is the first part of v5 patchset. The changelog of v5 is only coverd
the part1.

Features:
  * support specification riscv-v-spec-0.7.1.
  * support basic vector extension.
  * support Zvlsseg.
  * support Zvamo.
  * not support Zvediv as it is changing.
  * SLEN always equals VLEN.
  * element width support 8bit, 16bit, 32bit, 64bit.

Changelog:

v5
  * vector registers as direct fields in RISCVCPUState.
  * mov the properties to last patch.
  * check RVV in vs().
  * check if rs1 is x0 in vsetvl/vsetvli.
  * check VILL, EDIV, RESERVED fileds in vsetvl.
v4
  * adjust max vlen to 512 bits.
  * check maximum on elen(64bits).
  * check minimum on vlen(128bits).
  * check if rs1 is x0 in vsetvl/vsetvli.
  * use gen_goto_tb in vsetvli instead of exit_tb.
  * fixup fetch vlmax from rs2, not env->vext.type.
v3
  * support VLEN configure from qemu command line.
  * support ELEN configure from qemu command line.
  * support vector specification version configure from qemu command line.
  * only default on for "any" cpu, others turn on from command line.
  * use a continous memory block for vector register description.
V2
  * use float16_compare{_quiet}
  * only use GETPC() in outer most helper
  * add ctx.ext_v Property

LIU Zhiwei (4):
  target/riscv: add vector extension field in CPURISCVState
  target/riscv: implementation-defined constant parameters
  target/riscv: support vector extension csr
  target/riscv: add vector configure instruction

 MAINTAINERS                             |  1 +
 target/riscv/Makefile.objs              |  2 +-
 target/riscv/cpu.c                      |  7 +++
 target/riscv/cpu.h                      | 78 ++++++++++++++++++++++---
 target/riscv/cpu_bits.h                 | 15 +++++
 target/riscv/csr.c                      | 75 +++++++++++++++++++++++-
 target/riscv/helper.h                   |  2 +
 target/riscv/insn32.decode              |  5 ++
 target/riscv/insn_trans/trans_rvv.inc.c | 69 ++++++++++++++++++++++
 target/riscv/translate.c                | 17 +++++-
 target/riscv/vector_helper.c            | 53 +++++++++++++++++
 11 files changed, 312 insertions(+), 12 deletions(-)
 create mode 100644 target/riscv/insn_trans/trans_rvv.inc.c
 create mode 100644 target/riscv/vector_helper.c

-- 
2.23.0


Re: [PATCH v5 0/4] target-riscv: support vector extension part 1
Posted by Jim Wilson 4 years, 1 month ago
On 2/21/20 1:45 AM, LIU Zhiwei wrote:
> This is the first part of v5 patchset. The changelog of v5 is only coverd
> the part1.
> 
> Features:
>    * support specification riscv-v-spec-0.7.1.

I'm still concerned about versioning issues.  This implements an 
unofficial draft of the proposed RISC-V vector extension.  This draft is 
not compatible with the current draft, and will be even less compatible 
with the final official version of the vector spec.

The patch adds a version which is good, but there is only one check when 
qemu starts.  Probably something like 25% of these patches will be wrong 
for the official vector extension.  How are we going to handle this when 
someone submits patches for the official support?  It would be better if 
everything in these patches were conditional on the version number.  It 
might also be better if we stopped calling this the 'v' extension and 
maybe used another name like Xrvv071 to make it clear that it is an 
unofficial draft of the proposed vector spec.  Or maybe be we can use 
v0p7 but that isn't an officially supported extension name.

If this rvv 0.7.1 implementation is considered a temporary solution, 
maybe we can just remove all of this work when the official rvv spec if 
available?  But presumably it is better if we can have both this 
implementation and the official one, which means everything needs to be 
conditional or tied to an Xsomething extension name instead of the V 
extension name.

Jim

Re: [PATCH v5 0/4] target-riscv: support vector extension part 1
Posted by Alistair Francis 4 years, 1 month ago
On Wed, Feb 26, 2020 at 12:09 PM Jim Wilson <jimw@sifive.com> wrote:
>
> On 2/21/20 1:45 AM, LIU Zhiwei wrote:
> > This is the first part of v5 patchset. The changelog of v5 is only coverd
> > the part1.
> >
> > Features:
> >    * support specification riscv-v-spec-0.7.1.
>
> I'm still concerned about versioning issues.  This implements an
> unofficial draft of the proposed RISC-V vector extension.  This draft is
> not compatible with the current draft, and will be even less compatible
> with the final official version of the vector spec.

I wouldn't say this is an unofficial draft. v0.7.1 is an official and
tagged draft spec.

It is a draft spec and there is a new version (v0.8.0) and eventually
there will be a full (not draft release).

>
> The patch adds a version which is good, but there is only one check when
> qemu starts.  Probably something like 25% of these patches will be wrong
> for the official vector extension.  How are we going to handle this when
> someone submits patches for the official support?  It would be better if

The current plan (for all draft extensions) is to support just the
latest draft in QEMU. That is QEMU will have experimental support for
draft extension x at v0.1. When draft extension x is updated to v0.2
we will update the QEMU implementation (when we can) and only support
the v0.2.

We will continue updating and supporting the latest until the final
spec release, in which case we will then maintain the feature
following QEMU's deprecation policy.

While the spec is in a draft state the feature will be considered
experimental (hence the subject to change).

This way we can support and enable users to test on draft extensions,
but not spend all our time supporting draft extensions.

> everything in these patches were conditional on the version number.  It
> might also be better if we stopped calling this the 'v' extension and
> maybe used another name like Xrvv071 to make it clear that it is an
> unofficial draft of the proposed vector spec.  Or maybe be we can use
> v0p7 but that isn't an officially supported extension name.
>
> If this rvv 0.7.1 implementation is considered a temporary solution,
> maybe we can just remove all of this work when the official rvv spec if
> available?  But presumably it is better if we can have both this

That is generally the plan. When the final spec comes out this will be
updated and we will only support that.

> implementation and the official one, which means everything needs to be
> conditional or tied to an Xsomething extension name instead of the V
> extension name.

I agree that would be nice, but that is a lot of extra maintenance to
support a draft spec.

Considering most other projects won't accept draft specs I don't see
the huge appeal to maintain draft specs.

Alistair

>
> Jim

Re: [PATCH v5 0/4] target-riscv: support vector extension part 1
Posted by Jim Wilson 4 years, 1 month ago
On Wed, Feb 26, 2020 at 2:36 PM Alistair Francis <alistair23@gmail.com> wrote:
> On Wed, Feb 26, 2020 at 12:09 PM Jim Wilson <jimw@sifive.com> wrote:
> > If this rvv 0.7.1 implementation is considered a temporary solution,
> > maybe we can just remove all of this work when the official rvv spec if
> > available?  But presumably it is better if we can have both this
>
> That is generally the plan. When the final spec comes out this will be
> updated and we will only support that.

This solves my problem, but maybe creates one for Alibaba.  They have
apparently fabbed a chip using the 0.7.1 draft of the vector spec
proposal.  So for qemu to properly support their asic, the 0.7.1 draft
support will have to be retained.  But I think SiFive and everyone
else is waiting for the official spec before building asics with
vector support.  If Alibaba will update their processor as the spec
evolves, then maybe this isn't a problem for them.

Jim

Re: [PATCH v5 0/4] target-riscv: support vector extension part 1
Posted by Alistair Francis 4 years, 1 month ago
On Wed, Feb 26, 2020 at 3:40 PM Jim Wilson <jimw@sifive.com> wrote:
>
> On Wed, Feb 26, 2020 at 2:36 PM Alistair Francis <alistair23@gmail.com> wrote:
> > On Wed, Feb 26, 2020 at 12:09 PM Jim Wilson <jimw@sifive.com> wrote:
> > > If this rvv 0.7.1 implementation is considered a temporary solution,
> > > maybe we can just remove all of this work when the official rvv spec if
> > > available?  But presumably it is better if we can have both this
> >
> > That is generally the plan. When the final spec comes out this will be
> > updated and we will only support that.
>
> This solves my problem, but maybe creates one for Alibaba.  They have
> apparently fabbed a chip using the 0.7.1 draft of the vector spec
> proposal.  So for qemu to properly support their asic, the 0.7.1 draft
> support will have to be retained.  But I think SiFive and everyone
> else is waiting for the official spec before building asics with
> vector support.  If Alibaba will update their processor as the spec
> evolves, then maybe this isn't a problem for them.

That does seem unfortunate. I don't see a way for us to be able to
support previous draft spec versions though. That seems like a huge
amount of maintenance.

It shouldn't actually be that bad. Here is kind of what I'm imagining.

 - Vector extensions v0.7.1 are merged to QEMU
 - QEMU 5.x ships with v0.7.1 vector extensions (it seems unlikely
someone will upstream v0.8.0 before the 5.x release)

At some point in the future someone will update the experimental
support in QEMU from v0.7.1 to a fully released v1.0. In which case
there is still a release with the older experimental support which can
be used for the v0.7.1 if required.

Alistair

>
> Jim