Using sail-riscv version 0.9

[jeras]

The github actions are still running, but I got the tests to pass on my machine.

This commit handles issues when updating sail-riscv from 0.8 to 0.9, the details are evident from commit contents.

I have discussed the reasons regarding comments in the generated JSON configuration with sail-riscv developers, if you are interested:
riscv/sail-riscv#1411 (comment)

I have also spent a bit of time checking the remaining errors (while tests still pass) and if appears RISCOF is attempting to run some RV64 tests from riscof_work/database.yaml. I did not debug this further.

test_isa:  ['RV64IZimop']
   ERROR | Error in test: /home/izi/VLSI/NEORV32/neorv32-riscof-upstream/riscv-arch-test/riscv-test-suite/rv64i_m/Zimop/src/mop.r.0-01.S
Test Selected without the relevant extensions being available on DUT.
dut_isa:  RV32IMACUZicsr_Zicond_Zifencei_Zimop_Zca_Zcb_Zba_Zbb_Zbkb_Zbkc_Zbkx_Zbs_Zknd_Zkne_Zknh_Zksh_Zksed

[stnolting]

Thanks for updating! Give me some time to test this on my local machine as well.

I have discussed the reasons regarding comments in the generated JSON configuration with sail-riscv developers, if you are interested: riscv/sail-riscv#1411 (comment)

Thanks, I've already seen that. I can't wait for RISCOF to be kicked out of the flow... 😅

I have also spent a bit of time checking the remaining errors (while tests still pass) and if appears RISCOF is attempting to run some RV64 tests from riscof_work/database.yaml. I did not debug this further.

I have the same "error" with the current setup. I think the version of riscv_config used simply does not yet recognize the Zimop ISA extension.

[stnolting]

Unfortunately, this does not run on my local setup:

    INFO | ****** RISCOF: RISC-V Architectural Test Framework 1.25.3 *******
    INFO | using riscv_isac version : 0.18.0
    INFO | using riscv_config version : 3.18.3
    INFO | Reading configuration from: /mnt/n/projects/neorv32-riscof/config.ini
    INFO | Preparing Models
   ERROR | Error while importing sail_cSim.
   ERROR | No module named 'jsoncomment'

pip reports jsoncomment v0.4.2, so that shoould be fine. 🤔

[jeras]

You might use a different JSONC (JSON with comments) parser, I just used the first one I found instructions for.

I have made further changes in this branch: https://github.com/jeras/neorv32-riscof/tree/dut_makefile
The tests are passing: https://github.com/jeras/neorv32-riscof/actions/runs/19948082999
I would like to cleanup the commits before I create a pull request, and I have not updated instruction trace logging yet.

The main purpose of this changes was to speed up the runtime.
I was able to reduce the runtime of Github actions from 25~29 minutes to 6 minutes.
This was achieved primarily through 2 changes:

• Rewrite of plugin-neorv32/riscof_neorv32.py so it generates a Makefile instead of running gcc/GHDL directly from the Python code. To be able to run the tests in parallel, bin/hex/signature/log file paths must not be relative to the executable location (this would cause conflicts between tests). I have solved this by having all paths relative and passed to GHDL as generic overrides. By setting jobs=4 (github hosted runners) this reduce the runtime from 29 min to 9 min, It gets even faster on my local machine with more parallelism.
  I have yet to add file path support for instruction trace logging, but this is a change to the main NEORV32 repo.
• Moved handling of signature from the test firmware (the loop takes extra time to execute) to a memory dump withing the testbench. This required passing begin/end_signature symbols from the elf file to the testbench (generic overrides).

Other changes:

• Makefile now contains separate lines for each step instead of cd ...; gcc ...; objdump ...; ... this was done for both the DUT and reference.
• Removed the use of image_gen conversion, the bin file is directly loaded into the testbench, there is also a single array. There is a commit in the middle where a hex file was loaded into bit_vector(32-1 downto 0), but it used hexdump instead of image_gen.
• Made plugin-neorv32/env/model_test.h more similar to the sail counterpart, restored the tohost/fromhost symbols. Those are part of the HTIF protocol. While I was unable to find a proper document (there are a few, but I don't know which is used), HTIF is the approach shared by sail and spike, so kind of a standard.

TODO:

• instruction trace logging,

Issues:

• To me the system bus ack logic seemed redundant, so I just set ack = '1' but this caused the test pmpm_cfg_A_tor_zero to fail. I did not debug this in depth, but this could be an issue, for example the test passing due to some timeout.

I know I should create a separate issue for this, I am just lazy.

[stnolting]

I haven't fully understood your adjustments yet, but it looks pretty cool!

I'm following the updates from the architecture test SIG a little bit. Apparently, the "new framework" is coming out this year. I would then prefer to switch directly to this new framework.

I'll come back to this when I finally have vacation time. 😉

[jeras]

I could write a document (updated README.md) describing how my changes work, focusing on how they work while I can describe what changed in the pull request. I did spend some time on it, so I am willing to polish it a bit further, I would just like to know how wiling you would be to accept such a pull request. While I modified your testbench and Python plugin code significantly, the changes I did to the environment headers (assembly code) is to make it closer to the default test code (used by spike/sail), to it might make it easier to port to the next framework.

Could you give me some links on this "new framework"? I would like to read about it, maybe write a short essay on my experience with RISCOF, and what features would a framework need to be easy to use for students attempting to write their own RISC-V cores.

I also ported neorv32-riscof to the NVC VHDL simulator.
I hoped it might be faster than GHDL, but it was about 20% slower. Still there is a lot of VHDL-2019 support it can brag about.
The neorv32 code compiled without issues. I duplicated the testbench code, since:

• GHDL processes generics at run-time, so I pass testcode/signature/tracelog file paths at run-time using generic overrides.
• NVC processes generics at elaboration time, so using generics for file paths would mean rerunning elaboration for each test. Instead with NVC I used VHDL-2019 support for accessing shell environmental variables during runtime to pass the file paths and signature begin/end addresses.

[stnolting]

I could write a document (updated README.md) describing how my changes work, focusing on how they work while I can describe what changed in the pull request.

I don't think that's necessary for now. But thanks anyway!

I did spend some time on it, so I am willing to polish it a bit further, I would just like to know how wiling you would be to accept such a pull request.

I've looked at your commits. I don't understand everything yet, but I think some of it is pretty cool. I'm in the process of incorporating some of your changes (-> #443).

PRs are very welcome! 😉

While I modified your testbench and Python plugin code significantly, the changes I did to the environment headers (assembly code) is to make it closer to the default test code (used by spike/sail), to it might make it easier to port to the next framework.

That's pretty cool, because it alone makes the tests run much faster. However, I have to admit that I find some things quite cryptic:

# execute GHDL simulation
symbols_list = ['begin_signature', 'end_signature', 'tohost', 'fromhost']
symbol_generics = ' '.join([f'-g{symbol.upper()}=`grep -w {symbol} {symbols} | cut -c 1-8`' for symbol in symbols_list])

I'm currently working on a slightly different approach. Let's see how quickly the tests run with it...

Could you give me some links on this "new framework"?

It's all still a work in progress, but the first release is expected to come out this year. This is the development branch of the RISC-V Architectural Compatibility Test v4 (ACT4): https://github.com/riscv-non-isa/riscv-arch-test/tree/act4

I would like to read about it, maybe write a short essay on my experience with RISCOF, and what features would a framework need to be easy to use for students attempting to write their own RISC-V cores.

That would be great to have (for me too)!

I also ported neorv32-riscof to the NVC VHDL simulator.

I think the idea has been raised here before.. Maybe it's an interesting feature for the future - not necessarily from a performance perspective, but to "test" the code with a different simulator.

[jeras]

To parallelize test execution, a DUT makefile must be created, this is not possible if tests are executed withing the Python plugin itself. To avoid conflicts between the tests there are two approaches:

• At the beginning of the test, cd in the testcase folder. This is how the default RISCOF plugin does it. The downside is, within a Makefile target all commands have to bi concatenated on a single line with ; as separator, so all commands are executed withing the same testcase folder.
• All commands are called with absolute paths. This is the approach I used. There is not need to concatenate commands in the Makefile. While absolute paths are not appreciated in source code, this is not the case here. Only the generated Makefile contains absolute paths.

Regarding symbols, this is a generated Makefile.DUT-neorv32 target (it is rather long due to absolute paths):

.PHONY : TARGET0
TARGET0 :
	riscv-none-elf-gcc -march=rv32ia           -static -mcmodel=medany -fvisibility=hidden -nostdlib -nostartfiles -g          -T /neorv32-riscof/plugin-neorv32/env/link.ld          -I /neorv32-riscof/plugin-neorv32/env/          -I /neorv32-riscof/riscv-arch-test/riscv-test-suite/env /neorv32-riscof/riscv-arch-test/riscv-test-suite/rv32i_m/A/src/amoadd.w-01.S -o /neorv32-riscof/riscof_work/rv32i_m/A/src/amoadd.w-01.S/dut/main.elf  -DTEST_CASE_1=True -DXLEN=32 -mabi=ilp32  "-DSET_REL_TVAL_MSK=(((1<<CAUSE_MISALIGNED_LOAD)  | (1<<CAUSE_LOAD_ACCESS)      | (1<<CAUSE_MISALIGNED_STORE) | (1<<CAUSE_STORE_ACCESS)       ) & 0xFFFFFFFF)" 
	riscv-none-elf-objcopy -I elf32-little /neorv32-riscof/riscof_work/rv32i_m/A/src/amoadd.w-01.S/dut/main.elf -j .text -O binary /neorv32-riscof/riscof_work/rv32i_m/A/src/amoadd.w-01.S/dut/main.bin
	riscv-none-elf-nm /neorv32-riscof/riscof_work/rv32i_m/A/src/amoadd.w-01.S/dut/main.elf > /neorv32-riscof/riscof_work/rv32i_m/A/src/amoadd.w-01.S/dut/main.symbols
	../sim/ghdl_run.sh -gTEST_PATH=/neorv32-riscof/riscof_work/rv32i_m/A/src/amoadd.w-01.S/dut/ -gBEGIN_SIGNATURE=`grep -w begin_signature /neorv32-riscof/riscof_work/rv32i_m/A/src/amoadd.w-01.S/dut/main.symbols | cut -c 1-8` -gEND_SIGNATURE=`grep -w end_signature /neorv32-riscof/riscof_work/rv32i_m/A/src/amoadd.w-01.S/dut/main.symbols | cut -c 1-8` -gTOHOST=`grep -w tohost /neorv32-riscof/riscof_work/rv32i_m/A/src/amoadd.w-01.S/dut/main.symbols | cut -c 1-8` -gFROMHOST=`grep -w fromhost /neorv32-riscof/riscof_work/rv32i_m/A/src/amoadd.w-01.S/dut/main.symbols | cut -c 1-8`

The greps extract the HEX value of symbols begin/end_signature, to/fromhost from the main.symbols file (output of riscv-none-elf-nm main.elf > main.symbols) and passes it to the ghdl_run.sh script.
Withing the script this become HEX values (and a path to the testcase folder):

-gTEST_PATH=/NEORV32/neorv32-riscof/riscof_work/rv32i_m/A/src/amoadd.w-01.S/dut/ -gBEGIN_SIGNATURE=80002110 -gEND_SIGNATURE=80002340 -gTOHOST=80002400 -gFROMHOST=80002500

This is how I pass the begin/and signature addresses from the ELF file to the GHDL simulation without modifying the env/model_test.h.

You now pass the begin/and values to the testbench by writing them into specific addresses. I did the same in my first implementation. The problem I had with this approach is that changes model_test.h result in changes to the testcase executable (instructions are at a different address in memory. And if the testcase binaries for Sail/Spike and DUT are not exactly the same, I can't just compare the two instruction trace logs to find where during execution the two start to differ.

[stnolting]

I'm quite happy with the current speed so far. However, as soon as the new version of the architecture tests is available, we should take another close look at this and, if necessary, come back to your Makefile idea.

By the way, I would like to archive this repo (locking all further issues & PRs). The port of the architecture tests is now in the main repo: https://github.com/stnolting/neorv32/tree/main/sw/riscv-arch-test