-
Notifications
You must be signed in to change notification settings - Fork 637
[svsim] Trace options for the Verilator backend #4911
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
[svsim] Trace options for the Verilator backend #4911
Conversation
One question is if the default values for |
7c87e1c
to
558a2bd
Compare
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.
This generally seems great to me. I have some nits about how to structure the options for Verilator to be more flexible.
558a2bd
to
f6b4655
Compare
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 modulo nits.
Can't think of any reason for having them disabled, so not exposing them as options.
78f20bf
to
f67b3c2
Compare
maxArraySize
: Maximum array depth for tracingmaxWidth
: Maximum bit width for tracingtraceDepth
: Depth of tracingThe corresponding Verilator flags are only added if the above integer values are > 0.
Adds
TraceStyle.Fst
for generating FST traces.Removes the unused
filename
field fromTraceStyle
, which seemed very confusing. The trace file name (stem) is only configured throughCommonSimulationSettings.traceFileStem
.I guess, technically an API change due to the change of the
TraceStyle.Vcd
case class?Replaces #4910
Contributor Checklist
docs/src
?Type of Improvement
Desired Merge Strategy
Release Notes
Reviewer Checklist (only modified by reviewer)
3.6.x
,5.x
, or6.x
depending on impact, API modification or big change:7.0
)?Enable auto-merge (squash)
and clean up the commit message.Create a merge commit
.