-
Notifications
You must be signed in to change notification settings - Fork 613
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
Add simple API for generating testharnesses inline #4629
base: main
Are you sure you want to change the base?
Changes from 4 commits
db99d18
d96675c
4ffc80c
c9b887b
80f5a31
90f68a6
c18e660
a46103d
5e5bdd4
60f2e5f
a1a1a7f
502e0e9
1436ea2
d8aaaa7
3b24f28
941cc4b
0488111
2e715dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,71 @@ | ||||||
package chisel3.experimental | ||||||
|
||||||
import chisel3._ | ||||||
import chisel3.experimental.hierarchy.{Definition, Instance} | ||||||
|
||||||
/** Provides methods to elaborate additional parents to the circuit. */ | ||||||
trait ElaboratesParents { module: RawModule => | ||||||
private lazy val moduleDefinition = | ||||||
module.toDefinition.asInstanceOf[Definition[module.type]] | ||||||
|
||||||
/** Generate an additional parent around this module. | ||||||
* | ||||||
* @param parent generator function, should instantiate the [[Definition]] | ||||||
*/ | ||||||
def elaborateParentModule(parent: Definition[module.type] => RawModule with Public): Unit = | ||||||
afterModuleBuilt { Definition(parent(moduleDefinition)) } | ||||||
} | ||||||
|
||||||
/** Provides methods to build unit testharnesses inline after this module is elaborated. | ||||||
* | ||||||
* @tparam TestResult the type returned from each test body generator, typically | ||||||
* hardware indicating completion and/or exit code to the testharness. | ||||||
*/ | ||||||
trait HasTestsWithResult[TestResult] extends ElaboratesParents { module: RawModule => | ||||||
|
||||||
/** Given a [[Definition]] of this module, and a test body that takes an [[Instance]], | ||||||
* generate a testharness module that implements the test. | ||||||
* | ||||||
* The default behavior is a module with the following features: | ||||||
* | ||||||
* - A clock port | ||||||
* - A reset port with this module's reset type, or synchronous if unspecified | ||||||
* - The [[desiredName]] is "[[this.desiredName]] _ [[name]]". | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
*/ | ||||||
protected def generateTestHarness( | ||||||
testName: String, | ||||||
definition: Definition[module.type], | ||||||
body: Instance[module.type] => TestResult | ||||||
): RawModule with Public = new Module with Public { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The intent here is to allow the implementer to override it with something that generates a RawModule. |
||||||
override def resetType = module match { | ||||||
case module: Module => | ||||||
module.resetType match { | ||||||
case t @ Module.ResetType.Asynchronous => t | ||||||
case t @ Module.ResetType.Synchronous => t | ||||||
case _ => Module.ResetType.Synchronous | ||||||
} | ||||||
case _ => Module.ResetType.Synchronous | ||||||
} | ||||||
override val desiredName = s"${module.desiredName}_${testName}" | ||||||
val dut = Instance(definition) | ||||||
body(dut.asInstanceOf[Instance[module.type]]) | ||||||
tmckay-sifive marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
|
||||||
/** Generate a public module that instantiates this module. The default | ||||||
* testharness has clock and synchronous reset IOs and contains the test | ||||||
* body. | ||||||
* | ||||||
* @param body the circuit to elaborate inside the testharness | ||||||
*/ | ||||||
final def test(name: String)(body: Instance[module.type] => TestResult): Unit = { | ||||||
elaborateParentModule { | ||||||
generateTestHarness(name, _, body) | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
/** Provides methods to build unit testharnesses inline after this module is elaborated. | ||||||
* The test bodies do not communicate with the testharness and are expected to end the | ||||||
* simulation themselves. | ||||||
*/ | ||||||
trait HasTests extends HasTestsWithResult[Unit] { this: RawModule => } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
package chiselTests | ||
|
||
import chisel3._ | ||
import chisel3.util.Enum | ||
import chisel3.testers._ | ||
import chisel3.experimental.HasTests | ||
import chisel3.experimental.hierarchy._ | ||
import chisel3.experimental.HasTestsWithResult | ||
|
||
@instantiable | ||
class ModuleWithTests(ioWidth: Int = 32) extends Module with HasTests { | ||
@public val io = IO(new Bundle { | ||
val in = Input(UInt(ioWidth.W)) | ||
val out = Output(UInt(ioWidth.W)) | ||
}) | ||
|
||
io.out := io.in | ||
|
||
test("foo") { instance => | ||
instance.io.in := 3.U(ioWidth.W) | ||
assert(instance.io.out === 3.U) | ||
} | ||
|
||
test("bar") { instance => | ||
instance.io.in := 5.U(ioWidth.W) | ||
assert(instance.io.out =/= 0.U) | ||
} | ||
} | ||
|
||
trait HasTestsWithSuccess extends HasTestsWithResult[Bool] { module: RawModule => | ||
override protected def generateTestHarness( | ||
testName: String, | ||
definition: Definition[module.type], | ||
body: Instance[module.type] => Bool | ||
): RawModule with Public = | ||
new Module with Public { | ||
override val desiredName = s"${module.desiredName}_${testName}" | ||
val dut = Instance(definition) | ||
val result = body(dut.asInstanceOf[Instance[module.type]]) | ||
val success = IO(Output(Bool())) | ||
success := result | ||
} | ||
|
||
} | ||
|
||
@instantiable | ||
class ModuleWithTestsWithSuccess(ioWidth: Int = 32) extends Module with HasTestsWithSuccess { | ||
@public val io = IO(new Bundle { | ||
val in = Input(UInt(ioWidth.W)) | ||
val out = Output(UInt(ioWidth.W)) | ||
}) | ||
|
||
io.out := io.in | ||
|
||
test("foo") { instance => | ||
instance.io.in := 3.U(ioWidth.W) | ||
assert(instance.io.out === 3.U) | ||
WireInit(true.B) | ||
} | ||
|
||
test("bar") { instance => | ||
instance.io.in := 5.U(ioWidth.W) | ||
assert(instance.io.out =/= 0.U) | ||
WireInit(true.B) | ||
} | ||
} | ||
|
||
class HasTestsSpec extends ChiselFlatSpec with FileCheck { | ||
it should "generate a public module for each test" in { | ||
generateFirrtlAndFileCheck(new ModuleWithTests)( | ||
""" | ||
| CHECK: module ModuleWithTests | ||
| | ||
| CHECK: public module ModuleWithTests_foo | ||
| CHECK: input clock : Clock | ||
| CHECK: input reset : UInt<1> | ||
| CHECK: inst dut of ModuleWithTests | ||
| | ||
| CHECK: public module ModuleWithTests_bar | ||
| CHECK: input clock : Clock | ||
| CHECK: input reset : UInt<1> | ||
| CHECK: inst dut of ModuleWithTests | ||
""" | ||
) | ||
} | ||
|
||
it should "generate a public module for each test with a custom testharness" in { | ||
generateFirrtlAndFileCheck(new ModuleWithTestsWithSuccess)( | ||
""" | ||
| CHECK: module ModuleWithTestsWithSuccess | ||
| | ||
| CHECK: public module ModuleWithTestsWithSuccess_foo | ||
| CHECK: input clock : Clock | ||
| COM: CHECK: input reset : UInt<1> | ||
| CHECK: output success : UInt<1> | ||
| CHECK: inst dut of ModuleWithTestsWithSuccess | ||
| | ||
| CHECK: public module ModuleWithTestsWithSuccess_bar | ||
| CHECK: input clock : Clock | ||
| COM: CHECK: input reset : UInt<1> | ||
| CHECK: output success : UInt<1> | ||
| CHECK: inst dut of ModuleWithTestsWithSuccess | ||
""" | ||
) | ||
} | ||
} |
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.
Can this trait be inlined into its single extension trait,
HasTestWithResult
?Alternatively, this could be flattened in the other direction by making this a method on
RawModule
.Pre-emptively creating inheritance hierarchies is usually an anti-pattern. (Or: more strongly, inheritance hierarchies are usually an anti-pattern _unless they are architected and/or have some theoretical basis like Scala collections or Cats [1].)
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.
Yeah it could be flattened. The use case I had in mind that lead to this separation was that the user might have a component with a bunch of tests, while still wanting to elaborate a one-off parent module that is not the same as the testharnesses for all the tests.
So, I wanted to have an open-ended
elaborateParentModule
available to the user for that purpose. But, it felt strange to have that inHasTests
, since it's really a separate feature from unit tests; granted one that we can leverage to implement unit tests.That's how I got here. That said, I'm not very opinionated on the organization. So, if you think it's better Scala or more Chisel-like to flatten these traits, I will happily do so.