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

Add simple API for generating testharnesses inline #4629

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions src/main/scala/chisel3/experimental/inlinetest/package.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package chisel3.experimental.inlinetest
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, the name of this file shouldn't be package.scala, that is reserved for package objects, e.g. [1].

I'm not sure what to call this file since it has basically everything in the package in it, maybe just InlineTest.scala (still in directory src/main/scala/chisel3/experimental/inlinetest/) and we can break it up later if we see fit?


import chisel3._
import chisel3.experimental.hierarchy.{Definition, Instance}

/** Per-test parametrization needed to build a testharness that instantiates
* the DUT and elaborates a test body.
*
* @tparam M the type of the DUT module
* @tparam R the type of the result returned by the test body
*/
case class TestParameters[M <: RawModule, R](
/** The [[desiredName]] of the DUT module. */
dutName: String,
/** The user-provided name of the test. */
testName: String,
/** A Definition of the DUT module. */
dutDefinition: Definition[M],
/** The body for this test, returns a result. */
body: Instance[M] => R
)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an annoying detail but one we have to consider as library writers in Chisel.

What are the odds that we are going to have to evolve this type over time, like adding fields? I'm assuming it's high--if so, we should not make this a case class because case classes are nightmares for both source and binary compatibility.

Also, should users be able to create instances of this object or not? Maybe the constructor, factory apply, and copy methods should all be private (we won't have the latter two if we make this not a case class).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What are the odds that we are going to have to evolve this type over time, like adding fields? I'm assuming it's high--

Yeah, I would expect we will at some point.

if so, we should not make this a case class because case classes are nightmares for both source and binary compatibility.

Good to know. I made it a case class precisely because I wanted to avoid changes to the parameter list of the methods that take them.

I've changed it to a class for now.


/** An implementation of a testharness generator.
*
* @tparam M the type of the DUT module
* @tparam R the type of the result returned by the test body
*/
trait TestHarness[M <: RawModule, R] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Another nit but something to think about. Neither UnitTestHarness nor TestHarnessWithResultIO extend this trait which leads me to believe TestHarness is the wrong name for this TestHarness-providing-typeclass. I am not sure of the right name though. TestHarnessGenerator? @seldridge any ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think TestHarnessGenerator makes more sense. I've changed it to that pending other opinions.


/** Generate a testharness module given the test parameters. */
def generate(test: TestParameters[M, R]): RawModule with Public
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the type of M every really used? Or put another way--what is the use case for a TestHarness that only works for a particular kind of DUT Module? I suspect there is one, but maybe it would be good to have an example in the tests to drive the point home.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep I think it's useful to have when you want to reuse a testharness across DUTs where your testharness expects certain interfaces to be present on the DUT. Added an example.


object TestHarness {

/** The minimal implementation of a unit testharness. Has a clock input and a synchronous reset
* input. Connects these to the DUT and does nothing else.
*/
class UnitTestHarness[M <: RawModule](test: TestParameters[M, Unit]) extends Module with Public {
override def resetType = Module.ResetType.Synchronous
override val desiredName = s"test_${test.dutName}_${test.testName}"
val dut = Instance(test.dutDefinition)
test.body(dut)
}

implicit def unitTestHarness[M <: RawModule]: TestHarness[M, Unit] = new TestHarness[M, Unit] {
def generate(test: TestParameters[M, Unit]): RawModule with Public = new UnitTestHarness(test)
}
}

/** 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 HasTests[M <: RawModule] { module: M =>

/** A Definition of the DUT to be used for each of the tests. */
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]]
*/
protected final def elaborateParentModule(parent: Definition[module.type] => RawModule with Public): Unit =
afterModuleBuilt { Definition(parent(moduleDefinition)) }

/** 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
*/
protected final def test[R](testName: String)(body: Instance[M] => R)(implicit th: TestHarness[M, R]): Unit =
elaborateParentModule { moduleDefinition =>
val test = TestParameters[M, R](desiredName, testName, moduleDefinition, body)
th.generate(test)
}
}
100 changes: 100 additions & 0 deletions src/test/scala/chisel3/experimental/InlineTestSpec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package chiselTests

import chisel3._
import chisel3.util.Enum
import chisel3.testers._
import chisel3.experimental.inlinetest._
import chisel3.experimental.hierarchy._

class TestResultBundle extends Bundle {
val finish = Output(Bool())
val code = Output(UInt(8.W))
}

class TestHarnessWithResultIO[M <: RawModule](test: TestParameters[M, TestResultBundle]) extends Module {
override def resetType = Module.ResetType.Synchronous
override val desiredName = s"test_${test.dutName}_${test.testName}"
val dut = Instance(test.dutDefinition)
val result = IO(new TestResultBundle)
result := test.body(dut)
}

object TestHarnessWithResultIO {
implicit def enhancedTestHarness[M <: RawModule]: TestHarness[M, TestResultBundle] =
new TestHarness[M, TestResultBundle] {
def generate(test: TestParameters[M, TestResultBundle]): RawModule with Public =
new TestHarnessWithResultIO(test) with Public
}
}

@instantiable
class ModuleWithTests(ioWidth: Int = 32) extends Module with HasTests[ModuleWithTests] {
@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): Unit
}

test("bar") { instance =>
instance.io.in := 5.U(ioWidth.W)
assert(instance.io.out =/= 0.U): Unit
}

import TestHarnessWithResultIO._
test("enhanced") { instance =>
val result = Wire(new TestResultBundle)
val timer = RegInit(0.U)
timer := timer + 1.U
instance.io.in := 5.U(ioWidth.W)
val outValid = instance.io.out =/= 0.U
when(outValid) {
result.code := 0.U
result.finish := timer > 1000.U
}.otherwise {
result.code := 1.U
result.finish := true.B
}
result
}
}

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 test_ModuleWithTests_foo
| CHECK: input clock : Clock
| CHECK: input reset : UInt<1>
| CHECK: inst dut of ModuleWithTests
|
| CHECK: public module test_ModuleWithTests_bar
| CHECK: input clock : Clock
| CHECK: input reset : UInt<1>
| CHECK: inst dut of ModuleWithTests
|
| CHECK: public module test_ModuleWithTests_enhanced
| CHECK: input clock : Clock
| CHECK: input reset : UInt<1>
| CHECK: output result : { finish : UInt<1>, code : UInt<8>}
| CHECK: inst dut of ModuleWithTests
"""
)
}

it should "compile to verilog" in {
generateSystemVerilogAndFileCheck(new ModuleWithTests)(
"""
| CHECK: module test_ModuleWithTests_foo
| CHECK: module test_ModuleWithTests_bar
"""
)
}
}