jeremybennett
commented
2 years ago @DTowersM great work. This PR covers three areas:
- It adds support for WallyVerilog
- It fixes some bugs in the JSON output
- It fixes the new MD5sum issues.
Could I get you to break this out into three PRs. This means we could back-port the WallyVerilog support to the 1.0 branch, allowing us to get comparative official Embench 1.0 results. We can also potentially backport the JSON fixes. However md5sum is a proposed benchmark for Embench 2.0, so we wouldn't backport that.
You might like to take the opportunity to squash the commits for each of the three PRs into a single commit, since they will get squashed anyway, so you then take control of the commit message content. Above description of what you have done is great detail and really helpful. If you are able to add a GNU ChangeLog style listing explaining changes file-by-file to this, that is also helpful to future maintainers.
The technical content all looks good, so with this breakdown, we can merge all the changes.
Hi All!
Included in this pull request are configuration files and the python scripts needed for full Wally support.
We created a folder in /config/riscv32/boards/rv32wallyverilog/ which contains link.ld, boardsupport.c, boardsupport.h, config.cfg and startup/crt0.S and startup/dummy.S. The linker script makes sure the code begins at 0x80000000 and _start is at that address. The board support files are basically empty, we ended up defining the start trigger and stop trigger in the crt0.S file, with the boardsupport declaring them as external and doing little else. The crt0.S file is only needed in the speed tests, whereas the dummy.S file has dummy functions for start_trigger and stop_trigger for the size tests.
We noticed that we compile different code bases when we care about speed vs size. We kept flags that are true on both build types in the config.cfg file. Additionally, we wrote a makefile (makefile is in the wally repo, not the Embench repo) to launch build_all.py, to build the tests for speed and size in the bd_size and bd_speed directories. The makefile also takes care of launching the modelsim simulation, along with the speed_benchmark.py and size_benchmark.py python wrappers.
Additionally, We wrote a /pylib/run_wally.py which defines how to open the .sim.output file from modelsim. This file from modelsim contains the instret and cycles for start_trigger and stop_trigger, along with the return code. To maintain consistency/compatibility with the spike simulator, 1 indicates passing and 3 indicates failure. This python wrapper also prints the trigger info to the log file, while returning the time taken (assuming wally is running at 1MHz) to calculate the Embench score.
To calculate the time taken, we divide the cycles taken by the clock speed. Currently we have cpu_mhz defined as an input parameter to benchmark_speed.py. I noticed that cpu_mhz is also defined in the config.cfg file, in both the config/chips and config/board directories. Is there anyway to get this value into the benchmarking_speed.py script, or is having it as an input parameter an acceptable solution (it is what was done by the /pylib/run_stm32f4-discovery.py file).
Specifically, is having an external makefile an acceptable solution? How do we feel about printing the instret and CPI to the log file? What about the cpu-mhz being an input argument to benchmarking_speed rather than getting the value from config?
Looking forward to feedback to best cleanly integrate into the existing codebase