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 28 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
db99d18
add HasTests
tmckay-sifive Jan 16, 2025
d96675c
reformat
tmckay-sifive Jan 16, 2025
4ffc80c
make it easier to override testharness generation
tmckay-sifive Jan 17, 2025
c9b887b
add scaladoc
tmckay-sifive Jan 17, 2025
80f5a31
reformat
tmckay-sifive Jan 17, 2025
90f68a6
flatten traits
tmckay-sifive Jan 21, 2025
c18e660
refactor based on PR feedback
tmckay-sifive Jan 27, 2025
a46103d
reformat
tmckay-sifive Jan 27, 2025
5e5bdd4
fix inlinetest package organization
tmckay-sifive Jan 28, 2025
60f2e5f
rename to InlineTest.scala
tmckay-sifive Jan 31, 2025
a1a1a7f
make TestParameters not a case class
tmckay-sifive Jan 31, 2025
502e0e9
make TestParameters private[inlinetest]
tmckay-sifive Jan 31, 2025
1436ea2
rename TestHarness to TestHarnessGenerator
tmckay-sifive Jan 31, 2025
d8aaaa7
add example that utilizes DUT type in testharness
tmckay-sifive Jan 31, 2025
3b24f28
REVERTME: comment out failing test
tmckay-sifive Jan 31, 2025
941cc4b
scala format
tmckay-sifive Jan 31, 2025
0488111
Revert "REVERTME: comment out failing test"
tmckay-sifive Jan 31, 2025
2e715dd
update test
tmckay-sifive Jan 31, 2025
77291c9
Merge remote-tracking branch 'origin/main' into tmckay/inline-testing
tmckay-sifive Feb 4, 2025
81202f6
add license string
tmckay-sifive Feb 5, 2025
92e208b
remove extra space
tmckay-sifive Feb 5, 2025
abe0c6b
update tests per feedback
tmckay-sifive Feb 5, 2025
f7b5887
move test: src/test/scala/chisel3 -> src/test/scala/chiselTests
tmckay-sifive Feb 5, 2025
6cf7428
misc feedback
tmckay-sifive Feb 5, 2025
70b5b40
add TestHarnessRawModule and TestHarnessModule traits
tmckay-sifive Feb 5, 2025
659adc9
reformat
tmckay-sifive Feb 5, 2025
6760320
PR feedback
tmckay-sifive Feb 5, 2025
fb52653
Merge branch 'main' into tmckay/inline-testing
tmckay-sifive Feb 6, 2025
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
71 changes: 71 additions & 0 deletions src/main/scala/chisel3/experimental/HasTests.scala
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)) }
}
Copy link
Member

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].)

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 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 in HasTests, 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.


/** 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]]".
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
* - The [[desiredName]] is "[[this.desiredName]] _ [[name]]".
* - The [[desiredName]] is "[[this.desiredName]] _ [[testName]]".

*/
protected def generateTestHarness(
testName: String,
definition: Definition[module.type],
body: Instance[module.type] => TestResult
): RawModule with Public = new Module with Public {
Copy link
Member

Choose a reason for hiding this comment

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

A Module with Public being immediately cast to a RawModule with Public seems odd.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 => }
106 changes: 106 additions & 0 deletions src/test/scala/chisel3/experimental/HasTestsSpec.scala
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
"""
)
}
}
Loading