-
Notifications
You must be signed in to change notification settings - Fork 623
[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
base: main
Are you sure you want to change the base?
Conversation
One question is if the default values for |
.getOrElse(Paths.get("")) | ||
.toAbsolutePath | ||
|
||
def workingDirectory: Path = { |
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 looks like a better API to use generally should we deprecate val workingDirectoryPath
and suggest people use this?
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.
I can't think why not, so I moved getProjectRootOrCwd
to the Simulation
object so it's accessable elsewhere (and public).
I also added CHISEL_PROJECT_ROOT
env variable as an alternative way of specifying/detecting the project root. Then I used it for buildDir
of scalatest.TestingDirectory
.
Now scalatest.ChiselSim
build directory in my mill projects are created in the workspace folder (./build
) instead of being hidden under ./out/module_name/tests/testOnly.dest/sandbox/build
.
+ added `getProjectRootOrCwd` + added `workingDirectory` which should always be used (read) instead of `workingDirectoryPath` + replaced workingDirectoryPath usage with workingDirectory
Can't think of any reason for having them disabled, so not exposing them as options.
…Directory + added CHISEL_PROJECT_ROOT as an alternative for detectig project root + fixed tests to work with with both relative and absolute HasTestingDirectory.getDirectory path
7c87e1c
to
558a2bd
Compare
@jackkoenig @seldridge this is now ready for your reviews. |
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
.svsim: Try to figure out the project root directory in the presense of build-system sandboxing (e.g., as in the newer versions of mill). Introduces
workingDirectory
to be used instead ofworkingDirectoryPath
. I've had this issue with my own Chisel projects, but decided to sneak the fix in as it was causing problems while testing this PR.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
.