ibex icon indicating copy to clipboard operation
ibex copied to clipboard

core_ibex dv build system refactor

Open hcallahan-lowrisc opened this issue 2 years ago • 9 comments

[ This is a big PR (33 files changed, 2477 insertions(+), 1886 deletions(-)), so no rush to review, and feedback very much welcome. It would be helpful to just checkout and see how building works for you locally! ]

As well as completely removing the existing non-cosim flow, this commit significantly refactors the build system to be less reliant on the makefile. This is a continuation of previous refactoring such as this PR.

While we still use the Makefile, it is relegated to only providing scheduling and dependency calculations between the different build steps. This is possible by moving all of the build metadata into a file on-disk, which is populated at the start of a new regression, then read and written to by the different scripts executing the build. Each build step only needs to be passed the location of this metadata at the top-level, and it can then import all the information it requires to calculate and perform the next build stage.

As every stage has all of the information of previous stages available to it, it makes it very much more convenient to access useful data. This metadata object is stored as a serialized binary .pickle file, but is also exported as a human-readable yaml version for viewing.This should also aid integration into different build systems, as the dependency on Make is much weaker.

The file metadata.py and test_run_result.py contain the definitions for these metadata objects. metadata.py defines an object for the whole regression, while test_run_result.py defines objects for each individual test performed.

The file riscvdv_interface.py has been created to better isolate the interface with that project. The file setup_imports.py has been created to centralize the importing of python modules from other projects (riscvdv/ot_lowrisc_ip etc.). Existing python code has been tidied to better conform to PEP8 standard formatting, and to be more pythonic in general such as using pathlib.Path.

N.B. Some new python dependencies are added, so if running locally you will to update your python package set by running...

pip3 install -r python-requirements.txt

CI runs this command automatically so no change is needed for that.

hcallahan-lowrisc avatar Jul 14 '22 09:07 hcallahan-lowrisc

While running the following command:

make -j7 --keep-going IBEX_CONFIG=opentitan ITERATIONS=1 WAVES=0 COV=0

I encountered the following error:

riscv32-unknown-elf-gcc: error: ibex/dv/uvm/core_ibex/out/run/riscv_csr_test.2825/test.S: No such file or directory
riscv32-unknown-elf-gcc: fatal error: no input files
compilation terminated.
make: *** [Makefile:284: out/run/riscv_csr_test.2825/test.bin] Error 1

marnovandermaas avatar Jul 21 '22 15:07 marnovandermaas

I tried running this on my own system and here are a couple of comments.

We'll need to add the following Python 3 libraries as dependencies:

  • pathlib3x
  • typeguard
  • portalocker
  • typing_utils

I'm getting the following warning when running just $ make: WARNING:ibex_cmd:Rejecting test riscv_bitmanip_full_test, 'rtl_params' specified not compatible with ibex_config

The trr.yaml files and metadata directory seem to be generating successfully.

Thank you for looking at this @marnovandermaas . I've updated the original PR comment to mention the necessary python packages. They are added to the python-requirements.txt file as part of the PR but you have to update your local environment manually, so I left a comment with the command you need to run.

The "Rejecting test riscv_bitmanip_full_test" is just a more-verbose warning that tests are being excluded due to mismatching IBEX_CONFIG against the testlist. Not sure if this is an improvement, but I thought maybe being more-explicit would be useful.

While running the following command:

make -j7 --keep-going IBEX_CONFIG=opentitan ITERATIONS=1 WAVES=0 COV=0

I encountered the following error:

riscv32-unknown-elf-gcc: error: ibex/dv/uvm/core_ibex/out/run/riscv_csr_test.2825/test.S: No such file or directory
riscv32-unknown-elf-gcc: fatal error: no input files
compilation terminated.
make: *** [Makefile:284: out/run/riscv_csr_test.2825/test.bin] Error 1

Hmm interesting, I've not seen that error. Will investigate further.

Thanks again for starting to review this very-large changeset!

hcallahan-lowrisc avatar Jul 22 '22 09:07 hcallahan-lowrisc

Pushed a new commit 409e2dc which has some misc fixes, plus some improvements to regr.log, such as placing the summary percentage at the top of the file, and adding some lines of context around the log-line indicating the test failure. This should make the logfile more useful at a glance. regr_log_formatting

hcallahan-lowrisc avatar Jul 23 '22 17:07 hcallahan-lowrisc

One other thing I was thinking was automatically generating a test.dump file. I always end up running the following command when I'm debugging a test:

riscv32-unknown-elf-objdump -D test.o > test.dump

Would we be able to add this quality of life improvement to this build system overhaul?

marnovandermaas avatar Aug 01 '22 09:08 marnovandermaas

One other thing I was thinking was automatically generating a test.dump file. I always end up running the following command when I'm debugging a test:

riscv32-unknown-elf-objdump -D test.o > test.dump

Would we be able to add this quality of life improvement to this build system overhaul?

@marnovandermaas I actually already had scripts to run this locally, even had makefile targets for them in the PR but I didn't include the scripts. Pushed another commit, now you should be able to run make dump prettify to call the scripts and generate both a test.dump and a trace_pretty.log, which is a column-aligned version of the ibex trace.

hcallahan-lowrisc avatar Aug 01 '22 10:08 hcallahan-lowrisc

Rebased

hcallahan-lowrisc avatar Aug 10 '22 15:08 hcallahan-lowrisc

Rebased

hcallahan-lowrisc avatar Aug 11 '22 15:08 hcallahan-lowrisc

The previous rebase was bad (I missed part of the new test_done mechanism) so force-pushed again to fix it.

hcallahan-lowrisc avatar Aug 12 '22 13:08 hcallahan-lowrisc

Also some documentation going over the flow internals would be great, though not an immediate priority. Please create an issue to track the need for it (let's make it a V3 issue for now).

GregAC avatar Aug 12 '22 14:08 GregAC

Also some documentation going over the flow internals would be great, though not an immediate priority. Please create an issue to track the need for it (let's make it a V3 issue for now).

Noted in #1778. Thanks for the review :)

hcallahan-lowrisc avatar Aug 16 '22 12:08 hcallahan-lowrisc