Add support for CVA6 + HPDC#172
Conversation
It's necessary to analyze the proper value of the FIFO
|
This patch is ultimately pretty small but the history is quite long - could you squash it down to a single commit? |
|
Yup, I was planning on doing that once everything was set and ready, e.g. in case I need to add some more commits to apply feedback or update ariane or whatever. If you consider it necessary I can do that right now. |
There was a problem hiding this comment.
L15_BYTE_MASK_WIDHT -> L15_BYTE_MASK_WIDTH
There was a problem hiding this comment.
Changed, thanks for the heads-up
There was a problem hiding this comment.
I get this error (perl 5, version 26, subversion 1 (v5.26.1)): Unescaped left brace in regex is illegal here in regex; marked by <-- HERE in m/\${ <-- HERE ?(\w+)}?/ at piton/tools/src/sims/sims,2.0 line 1165
Shouldn't it be $line =~ s/\$\{?(\w+)\}?/$ENV{$1}/g ; ?
There was a problem hiding this comment.
Hmmm that doesn't work for me, and {} are not special characters in regex.
There was a problem hiding this comment.
I don't get this error anymore, which I believe is because I have since updated my system. My apologies!
There was a problem hiding this comment.
No worries, glad it's resolved
There was a problem hiding this comment.
I don't understand, why should that be updated?
There was a problem hiding this comment.
I was getting some errors because verilator is no longer in that directory (and I am using verilator-v5.008, as i believe this PR also aims to do in commit 44c420)
There was a problem hiding this comment.
Ah I see, they have upgraded the verilator version in upstream, I'll update it here too.
To be honest, I don't like these scripts installing tools and stuff, I like using my own and that's why I didn't catch this error, but I'll fix it regardless
|
Ariane patches have been merged, let me know how we can proceed with this! |
Jbalkind
left a comment
Jbalkind
left a comment
There was a problem hiding this comment.
Have you run the OpenPiton CI scripts with OST1 too? Have you checked that the changes don't break the other cores?
| CFLAGS=-mabi=lp64d -Tlinker.ld -nostdlib -static -Wl,--no-gc-sections | ||
|
|
||
| ifeq "$(GCC_VERSION_GTEQ_12)" "1" | ||
| CFLAGS += -march=rv64imafd_zicsr |
There was a problem hiding this comment.
not sure that I care about making this conditional - probably everyone has GCC newer than 12 at this point
There was a problem hiding this comment.
You'd be surprised... Although I guess the changes were done a while ago
There was a problem hiding this comment.
This was added something like 4-5 years ago so I'm fine with just making it unconditional as a change
| LDFLAGS = -nostdlib -nodefaultlibs -nostartfiles | ||
|
|
||
| ifeq "$(GCC_VERSION_GTEQ_12)" "1" | ||
| CFLAGS += -march=rv64ima_zicsr |
| -flist=$DV_ROOT/design/chip/jtag/rtl/Flist.jtag | ||
| -flist=$DV_ROOT/design/chip/tile/rtl/Flist.tile | ||
| -flist=$DV_ROOT/design/chip/tile/rtap/rtl/Flist.rtap | ||
| -flist=$DV_ROOT/design/chip/tile/l15/rtl/Flist.l15 |
There was a problem hiding this comment.
I'm hesitant about reordering this as I have to spend quite a long time to find the correct order for particular tools. Why is this one being moved specifically?
There was a problem hiding this comment.
I have changed the order because Flist.l15 contains the l15 package that CVA6 then uses for its configuration and adapters.
There was a problem hiding this comment.
Right but that can break things across tools. This order was carefully set to make sure it works across VCS, Questa, Incisive, etc. Have you checked it hasn't broken anything across tools?
| push (@{$opt{diaglist_cpp_args}}, "-DRISCV32") if ($opt{pico} || $opt{pico_het}) ; | ||
| push (@{$opt{diaglist_cpp_args}}, "-DRISCV64") if ($opt{ariane}) ; | ||
|
|
||
| if ($opt{ariane}) { |
There was a problem hiding this comment.
Can this one be put in another place where config_rtl are being set?
| $build_cmd .= "-CFLAGS -I$dv_root/tools/pli/iop " ; | ||
| $build_cmd .= "-CFLAGS -I$dv_root/tools/verilator " ; | ||
|
|
||
| if ($opt{log_all}) { |
There was a problem hiding this comment.
is this logging really important enough to be PRd here?
There was a problem hiding this comment.
It was for my debugging convenience -- we can split it up
| } | ||
| } | ||
|
|
||
| # Run the verilator --version command and capture the output |
There was a problem hiding this comment.
I am ok with us moving to assuming verilator 5 going forward. Is that what is built by default in cva6 now?
There was a problem hiding this comment.
I also have a branch here with other changes for timing mode for v5 that we could consider pulling in if we know the right macro for timing vs no timing https://github.com/Jbalkind/openpiton/tree/verilator-5
There was a problem hiding this comment.
What do you mean the right macro? Also, removing the my_top.cpp has some drawbacks as you have less control over the "runtime", in my experience. Things like not dumping the waveform when you do ctrl+c, for example .
| [submodule "piton/design/chip/tile/sparc"] | ||
| path = piton/design/chip/tile/sparc | ||
| url = git@github.com:openpiton/ost1-core.git | ||
| [submodule "piton/design/chip/tile/ariane"] |
There was a problem hiding this comment.
This isn't a valid change to .gitmodules
There was a problem hiding this comment.
Actually can we just remove this? The repo should be redirecting already and I'd rather not force every user to git submodule sync
No, only the CVA6 + HPDC configuration, I understood that was the idea. What do you think we should test? |
|
We should check at least that the OST1 (small ones like tile1_mini and tile2_mini are likely sufficient) and WT_DCACHE Ariane regressions still work |
4 participants
This Pull Request adds support for using the HPDC in OpenPiton. To do so, I have added and updated the L1.5 adapters and hpdcache cache subsystems.
Because previously the L1.5 interface types & enums were in the wt_cache pkg and adapter files, I have moved the typedefs to a l15_pkg in this repository. This allows any cache (and any core) to interface with the L1.5.
I have tested the changes and they're passing physical and virtual single core tests. I am running more tests but I thought I'd open the merge requests already to get any possible discussions out of the way earlier.
When openhwgroup/cva6#3134 is accepted I will change the submodule paths and this will be ready to be merged.
This merge requests also contains some misc. fixes by me to provide support for my testing environment, mainly verilator 5+, waveforms, multithreaded compilation of bootrom, newer riscv compiler fixes, etc...
CC: @Jbalkind