Add ChiselSim Settings.defaultTest for SimulationTestHarnessInterface modules #5192
Conversation
| elaboratedModule: ElaboratedModule[A] | ||
| ) = { | ||
| val port = elaboratedModule.portMap(get(elaboratedModule.wrapped)).name | ||
| val port = lookupSignal(get(elaboratedModule.wrapped), elaboratedModule) |
There was a problem hiding this comment.
I guess this is technically a behavior change. Lmk if you'd like me to land this separately @jackkoenig.
There was a problem hiding this comment.
It's more of a bugfix than behavior change, but it's fine.
| chiselOpts: Array[String] = Array.empty, | ||
| firtoolOpts: Array[String] = Array.empty, | ||
| settings: Settings[T] = Settings.defaultRaw[T], | ||
| settings: Settings[T] = Settings.defaultTest[T], |
There was a problem hiding this comment.
Behavior change here too
There was a problem hiding this comment.
This I missed, what's the change in behavior?
There was a problem hiding this comment.
Before this PR, STOP_COND, PRINTF_COND, etc. will not be set due to the use of defaultRaw. Now, they will be set to !init. As you point out, maybe a this is more of a bug fix than a behavior change but I feel like the the line is blurry 😄
jackkoenig
left a comment
jackkoenig
left a comment
There was a problem hiding this comment.
Nice tests and good catch with the view support.
| io.Source | ||
| .fromFile( | ||
| FileSystems | ||
| .getDefault() | ||
| .getPath(implicitly[HasTestingDirectory].getDirectory.toString, "workdir-verilator", "Makefile") | ||
| .toFile | ||
| ) | ||
| .mkString | ||
| .fileCheck()( |
There was a problem hiding this comment.
A nit but,
FilesSystems.getDefault().getPath is the same as just Paths.get and I think the latter is much nicer generally speaking.
The split line is also a bit ugly, I'd use a val for the makefile path and this should be a lot simpler:
| io.Source | |
| .fromFile( | |
| FileSystems | |
| .getDefault() | |
| .getPath(implicitly[HasTestingDirectory].getDirectory.toString, "workdir-verilator", "Makefile") | |
| .toFile | |
| ) | |
| .mkString | |
| .fileCheck()( | |
| val makefile = Paths.get(implicitly[HasTestingDirectory].getDirectory.toString, "workdir-verilator", "Makefile") | |
| io.Source.fromFile(makefile).mkString.fileCheck()( |
There was a problem hiding this comment.
After writing this comment, I see that it follows the style of the rest of the file. So I think it's fine, but consider yourself on notice @seldridge 👀
|
Whoops look like some Scala 3-related changes have landed since I branch. Will fix those. EDIT: Nvm, just missing fields in Settings. |
2 participants
Contributor Checklist
docs/src?Type of Improvement
Desired Merge Strategy
Release Notes
Settings.defaultTest[A <: RawModule with SimulationTestHarnessInterface]. This is useful for specifyingSTOP_COND,ASSERT_VERBOSE_COND, andPRINTF_CONDautomatically for simulations of aSimulationTestHarnessInterfacemodule.Reviewer Checklist (only modified by reviewer)
3.6.x,5.x, or6.xdepending on impact, API modification or big change:7.0)?Enable auto-merge (squash)and clean up the commit message.Create a merge commit.