Alistair Francis <alistai...@gmail.com> writes:
> On Thu, Aug 29, 2019 at 5:05 AM liuzhiwei <zhiwei_...@c-sky.com> wrote: >> >> On 2019/8/29 上午5:34, Alistair Francis wrote: >> > On Wed, Aug 28, 2019 at 12:04 AM liuzhiwei <zhiwei_...@c-sky.com> wrote: >> >> Change-Id: I3cf891bc400713b95f47ecca82b1bf773f3dcb25 >> >> Signed-off-by: liuzhiwei <zhiwei_...@c-sky.com> >> >> --- >> >> fpu/softfloat.c | 119 + >> >> include/fpu/softfloat.h | 4 + >> >> linux-user/riscv/cpu_loop.c | 8 +- >> >> target/riscv/Makefile.objs | 2 +- >> >> target/riscv/cpu.h | 30 + >> >> target/riscv/cpu_bits.h | 15 + >> >> target/riscv/cpu_helper.c | 7 + >> >> target/riscv/csr.c | 65 +- >> >> target/riscv/helper.h | 354 + >> >> target/riscv/insn32.decode | 374 +- >> >> target/riscv/insn_trans/trans_rvv.inc.c | 484 + >> >> target/riscv/translate.c | 1 + >> >> target/riscv/vector_helper.c | 26563 >> >> ++++++++++++++++++++++++++++++ >> >> 13 files changed, 28017 insertions(+), 9 deletions(-) >> >> create mode 100644 target/riscv/insn_trans/trans_rvv.inc.c >> >> create mode 100644 target/riscv/vector_helper.c >> >> >> > Hello, >> > >> > Thanks for the patch! >> > >> > As others have pointed out you will need to split the patch up into >> > multiple smaller patches, otherwise it is too hard to review almost >> > 30,000 lines of code. >> >> Hi, Alistair >> >> I'm so sorry for the inconvenience. It will be a patch set with a cover >> letter in V2. > > No worries. > >> >> > Can you also include a cover letter with your patch series describing >> > how you are testing this? AFAIK vector extension support isn't in any >> > compiler so I'm assuming you are handwriting the assembly or have >> > toolchain patches. Either way it will help if you can share that so >> > others can test your implementation. >> >> Yes, it's handwriting assembly. The assembler in Binutils has support >> Vector extension. First define an function test_vadd_vv_8 in assembly >> and then it can be called from a C program. >> >> The function is something like >> >> /* vadd.vv */ >> TEST_FUNC(test_vadd_vv_8) >> vsetvli t1, x0, e8, m2 >> vlb.v v6, (a4) >> vsb.v v6, (a3) >> vsetvli t1, a0, e8, m2 >> vlb.v v0, (a1) >> vlb.v v2, (a2) >> vadd.vv v4, v0, v2 >> vsb.v v4, (a3) >> ret >> .size test_vadd_vv_8, .-test_vadd_vv_8 > > If possible it might be worth releasing the code that you are using for > testing. > >> >> It takes more time to test than to implement the instructions. Maybe >> there is some better test method or some forced test cases in QEMU. >> Could you give me some advice for testing? > > Richard's idea of risu seems like a good option. > > Thinking about it a bit more we are going to have other extensions in > the future that will need assembly testing so setting up a test > framework seems like a good idea. I am happy to help try and get this > going as well. tests/tcg already has the bits you need for both linux-user and system based testing. The main problem is getting a version of gcc that is new enough to emit the newer instructions. I recently updated the images to buster so gcc is pretty recent now (8.3). I did start down the road of a general "op" test frame work which tried to come up with a common framework/boilerplate so all you needed to do was supply a new function (possible with a hex encoded instruction) and a list of expected inputs and outputs: https://github.com/stsquad/qemu/commits/testing/generic-op-tester I suspect it was over engineered but perhaps it would be worth reviving it (or something like it) to make adding a simple single instruction test case with minimal additional verbiage? > > Alistair > >> >> Best Regards, >> >> Zhiwei >> >> > Alex and Richard have kindly started the review. Once you have >> > addressed their comments and split this patch up into smaller patches >> > you can send a v2 and we can go from there. >> > >> > Once again thanks for doing this implementation for QEMU! >> > >> > Alistair >> > -- Alex Bennée