search

tum-ei-eda / etiss

Extendable Translating Instruction Set Simulator
https://tum-ei-eda.github.io/etiss/
Other
29 stars 36 forks source link

Add RISC-V Vector Extension Support #76

Open fpedd opened 3 years ago

fpedd commented 3 years ago

This is a work in progress replacement for #71...

Some points that need to be considered:

  1. Currently, the VLE.U and VSU.U (lines 16460-16640 in ArchImpl/RISCV/RISCVArch.cpp) cause some sort of conflict in the ETISS instruction decoder which results in uninitialized nodes in the instruction set tree, similar to here #74. This results, in contrast to the uninitialized nodes in referenced issue, in segmentation faults when running ETISS. Thus, these instructions are commented out for now.
  2. While a good part of the softvector library, which implements the logic behind most of the vector instructions, appears to have decent test coverage, there are currently no ISA level tests for the vector instructions. Ideally, one would run some amount of ISA level tests, similar to the ones provided here or here. The problem however is, that the mentioned tests are using vector instructions v0.9 (and above?).
  3. It appears that our CoreDSL (see below) adheres to v0.7. It might be beneficial to migrate to v0.9+, especially when considering that the tests use v0.9 and that muriscv_nn should be up-to-date while being able to run on ETISS.
  4. There are also many more vector instructions to support (look here for a compact summary).
  5. This CoreDSL was fed into m2-isa-r to produce the vector instructions. However, it was necessary to copy the vector instructions from the files generated by m2-isa-r into the existing arch files, as the files currently generated by m2-isa-r are not fully compatible with ETISS.

EDIT 1: Regarding 1. The RISCVArch.cpp (beware, the file is a pain to work with as it has close to a million lines) in the v-ext-support branch doesn't seem to include the VLE.U and VSU.U instructions either. So this could be the reason why the issue is only now emerging. The ml_on_mcu flow is currently based on ETISS with v-ext-support when requesting vector support (the muriscv_nn library inst using any vector load/stores as of now).

DanMueGri commented 3 years ago

To 1. I think this is actually interesting, seems we are now "stressing" the ETISS decoder implementation so some bugs surface. Definitely we need to have alook into this but that would rather be something for @rafzi @wysiwyng .

  1. Defintely a good idea.

  2. Yes, we defintely should target the latest Vector release and not spend time on older V versions. Was there not a 1.X Version already?

  3. I woulkd postpone it as we only use V for ML workloads atm.

  4. I think you should report this to @wysiwyng , finally we should run the fully generated files.

fpedd commented 3 years ago

  1. Yes, there already is a v1.0-rc1 (released June this year). While it still is a release candidate, the following (taken from the PDF spec) seems pretty ensuring:

    When finally approved and the release candidate tag is removed, version 1.0 is intended to be sent out for public review as part of the
RISC-V International ratification process. Version 1.0 is also considered stable enough to begin developing toolchains, functional
simulators, and initial implementations, including in upstream software projects, and is not expected to have major functionality
changes except if serious issues are discovered during ratification. Once ratified, the spec will be given version 2.0.

    So it probably was a good idea to not invest too much time into implementing other prior 0.x versions of the vector standard. But now could be a good time to go forward with adopting the (hopefully mostly stable) v1.0.

  2. Yes, of course. Only focussing on vector instructions directly beneficial to the ml flow and the muriscv_nn library is most likely the only reasonable approach.

  3. I already talked to @wysiwyng. He is working on this.

fpedd commented 3 years ago

While the RISC-V vector extension has already defined a version 1.0, there appears to be no tool support for it as of now. Thus, @rafzi rightfully suggested postponing any further work on implementing the version 1.0 vector extension until one can actually assemble and test version 1.0 vector instructions. This PR is not directly affected by this. But it directly relates to my remarks made in the original comment for this PR.

We are also waiting for this PR to close this issue in order for the Windows build to hopefully succeed here.

fpedd commented 3 years ago

Reading this makes me wonder if we should consider LLVM. @rafzi what do you think?