qtrvsim icon indicating copy to clipboard operation
qtrvsim copied to clipboard

Add Instruction Test

Open trdthg opened this issue 10 months ago • 6 comments

Related to #7

This demo currently adds some test cases for instruction_to_str(), and also includes a demo function to generate test cases

  • Most of the test cases are generated automatically, one for each instruction (except for the first 7, which I first tried to write by hand, but it was too inefficient).
  • Registers, Imm, addresses are simply set to 1 (or other values as needed)

@jdupak

  • Do you have any suggestions on the method I'm using here, or provide more test topics?
  • What about programloader tests, should we now simply hand write tests to cover as many instructions and section types as possible, or look for ways to reuse the sail-riscv test set? (The latter seems pretty complicated.)

  • [x] split test case generator into instruction.test.gendata.cpp
  • [x] code_from_string added
  • [x] making the inputs more random
  • inst coverage
    • [x] normal inst
    • [x] alias
    • [x] pseudo (handwritten)
    • [x] csr
  • bugs related
    • [ ] https://github.com/cvut/qtrvsim/issues/127
    • [x] https://github.com/cvut/qtrvsim/pull/126
    • [x] https://github.com/cvut/qtrvsim/pull/125
    • [x] https://github.com/cvut/qtrvsim/pull/132
    • [x] https://github.com/cvut/qtrvsim/pull/133
  • [ ] clean debug

trdthg avatar Apr 18 '24 18:04 trdthg

I will address the other questions shortly.

jdupak avatar Apr 18 '24 19:04 jdupak

What about programloader tests, should we now simply hand write tests to cover as many instructions and section types as possible, or look for ways to reuse the sail-riscv test set? (The latter seems pretty complicated.)

I don't think we need to dig into the instructions here. We need to test the loading functionality. I think it would be nice to come up with one executable with various sections and then "randomly" check the contents of the memory. As for sail-riscv, as far as I understand, they are just binaries of the official risc-v test, which we are running already (see the CI).

jdupak avatar Apr 18 '24 20:04 jdupak

Do you have any suggestions on the method I'm using here, or provide more test topics?

I like your approach. Since you have the pairs already generated, I would love to add also the other direction (code_from_string).

After that, there are many commented-out MIPS tests in the core.test.cpp. Converting them to RISC-V would be great.

jdupak avatar Apr 18 '24 20:04 jdupak

We will be able to close #7 once all QSKIP calls are gone.

jdupak avatar Apr 18 '24 20:04 jdupak

Good work so far. Some notes.

  • You will need to clean up the commits a bit.
  • Try making the inputs more random. x1 and immediate zero is not so great test.

jdupak avatar May 02 '24 09:05 jdupak

Completely refactored

Now instruction.test.gendata.cpp iterates through all the instructions on instruction_map, tries to generate 100 (code and string_data)pair for each, then checks to_str and code_from_string and fails if they don't match.

The current generation logic relies on arg_desc's min and max.

This is an executable file, the output looks like this:

...
inst: [jalr] 

  - [ 0]    code: [  fb268267] str: [             jalr x4, -78(x13)]
  - [ 1]    code: [  7dce8a67] str: [           jalr x20, 2012(x29)]
  - [ 2]    code: [  cfc78ee7] str: [           jalr x29, -772(x15)]
  ...

inst: [j] 

  - [ 0]    code: [  e32f006f] str: [                     j -0xf9ce]
  ...
...

Since it can't generate a fixed test case every time you run this script, the file name shouldn't be gendata anymore. I tried to move the code from main to instruction.test.cpp, but fails.

Because there are too many structures used here that instruction.cpp doesn't expose (e.g. ArgumentDesc, C_inst_map, fill_argdesbycode ...), So I include instruction.cpp directly.

@jdupak could you give me some advise?

trdthg avatar Jun 26 '24 02:06 trdthg