Skip to content
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

Merged
merged 7 commits into from
May 10, 2023
Merged

Conversation

abejgonzalez
Copy link
Contributor

@abejgonzalez abejgonzalez commented Mar 12, 2023

This removes the need for emulator.cc (a copy of Rocket Chip's emulator.cc only needed for Verilator) and instead uses the TestDriver.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:

  • Bug fix
  • New feature
  • Other enhancement

Impact:

  • RTL change
  • Software change (RISC-V software)
  • Build system change
  • Other

Contributor Checklist:

  • Did you set main as the base branch?
  • Is this PR's title suitable for inclusion in the changelog and have you added a changelog:<topic> label?
  • Did you state the type-of-change/impact?
  • Did you delete any extraneous prints/debugging code?
  • Did you mark the PR with a changelog: label?
  • (If applicable) Did you update the conda .conda-lock.yml file if you updated the conda requirements file?
  • (If applicable) Did you add documentation for the feature?
  • (If applicable) Did you add a test demonstrating the PR?
  • (If applicable) Did you mark the PR as Please Backport?

@abejgonzalez abejgonzalez changed the title Bump Verilator and Verilog-as-Top Bump Verilator and use TestDriver.v as top Mar 13, 2023
@abejgonzalez
Copy link
Contributor Author

If this PR is fine, we can merge this in pre-release... but otherwise we can wait until after the release.

@abejgonzalez abejgonzalez self-assigned this Mar 13, 2023
Copy link
Contributor

@jerryz123 jerryz123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@abejgonzalez
Copy link
Contributor Author

This breaks FireSim. Postponing to after release.

@vighneshiyer
Copy link
Contributor

Is there any performance degradation when using Verilog top vs C++ top with Verilator 5?

@abejgonzalez
Copy link
Contributor Author

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

@abejgonzalez
Copy link
Contributor Author

My extremely scientific setup for performance regressions:

Config: MegaBoomConfig running run-bmark-tests (serially without any Scala compilation):

w/ 5.006 = 37m44s
w/ 4.226 = 41m7s

(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.

@abejgonzalez abejgonzalez merged commit f111e2d into main May 10, 2023
46 of 48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants