-
Notifications
You must be signed in to change notification settings - Fork 678
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump Verilator and use TestDriver.v
as top
#1398
Conversation
TestDriver.v
as top
b035e23
to
8f1ce3c
Compare
8f1ce3c
to
9534975
Compare
If this PR is fine, we can merge this in pre-release... but otherwise we can wait until after the release. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This breaks FireSim. Postponing to after release. |
Is there any performance degradation when using Verilog top vs C++ top with Verilator 5? |
I'll run a few quick tests. But I can't see anything online about performance regressions |
My extremely scientific setup for performance regressions: Config: w/ 5.006 = 37m44s (Note that the 4.226 version might of had some contention (I was running a Scala compile on another tmux window)) IMO there isn't a large difference between the two so I think it's safe to say performance difference is negligible. |
This removes the need for
emulator.cc
(a copy of Rocket Chip'semulator.cc
only needed for Verilator) and instead uses theTestDriver.v
file given by RC (this standardizes the simulator interface between VCS and Verilator). Additionally, this bumps to the newest version of Verilator for misc. bug fixes and perf. improvements. I've verified that the VCD and FST dumped out of this new flow works properly.Related PRs / Issues:
Type of change:
Impact:
Contributor Checklist:
main
as the base branch?changelog:<topic>
label?changelog:
label?.conda-lock.yml
file if you updated the conda requirements file?Please Backport
?