Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
46 changes: 43 additions & 3 deletions src/main/scala/chisel3/simulator/Settings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

package chisel3.simulator

import chisel3.{Data, Module, RawModule}
import chisel3.{Data, Module, RawModule, SimulationTestHarnessInterface}
import chisel3.experimental.dataview.reifySingleTarget
import svsim.CommonCompilationSettings.VerilogPreprocessorDefine
import svsim.Workspace

Expand All @@ -27,6 +28,16 @@ object MacroText {

}

/** Given a [[Data]] and its parent [[ElaboratedModule]], lookup the underlying hardware. */
private def lookupSignal[A <: RawModule](data: Data, elaboratedModule: ElaboratedModule[A]): String = {
val reified = reifySingleTarget(data).getOrElse {
throw new IllegalArgumentException(
s"Cannot use $data as a macro signal because it does not map to a single Data."
)
}
elaboratedModule.portMap(reified).name
}

/** A macro that will return macro text with the name of a signal in the design
* under test
*
Expand All @@ -38,7 +49,7 @@ object MacroText {
macroName: String,
elaboratedModule: ElaboratedModule[A]
) = {
val port = elaboratedModule.portMap(get(elaboratedModule.wrapped)).name
val port = lookupSignal(get(elaboratedModule.wrapped), elaboratedModule)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess this is technically a behavior change. Lmk if you'd like me to land this separately @jackkoenig.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's more of a bugfix than behavior change, but it's fine.

VerilogPreprocessorDefine(macroName, s"${Workspace.testbenchModuleName}.$port")
}

Expand All @@ -55,7 +66,7 @@ object MacroText {
macroName: String,
elaboratedModule: ElaboratedModule[A]
) = {
val port = elaboratedModule.portMap(get(elaboratedModule.wrapped)).name
val port = lookupSignal(get(elaboratedModule.wrapped), elaboratedModule)
VerilogPreprocessorDefine(macroName, s"!${Workspace.testbenchModuleName}.$port")
}

Expand Down Expand Up @@ -230,6 +241,35 @@ object Settings {
libraryPaths = Seq.empty
)

/** Return a default [[Settings]] for a [[SimulationTestHarnessInterface]]. Macros will be set to
* disable [[chisel3.assert]]-style assertions using the [[SimulationTestHarnessInterface]]'s init
* port.
*
* Note: this _requires_ that an explicit type parameter is provided. You
* must invoke this method like:
*
* {{{
* Settings.default[Foo]
* }}}
*
* If you invoke this method like the following, you will get an error:
*
* {{{
* Settings.default
* }}}
*/
final def defaultTest[A <: RawModule with SimulationTestHarnessInterface]: Settings[A] = new Settings[A](
verilogLayers = LayerControl.EnableAll,
assertVerboseCond = Some(MacroText.NotSignal(get = _.init)),
printfCond = Some(MacroText.NotSignal(get = _.init)),
stopCond = Some(MacroText.NotSignal(get = _.init)),
plusArgs = Seq.empty,
enableWavesAtTimeZero = false,
randomization = Randomization.random,
libraries = Seq.empty,
libraryPaths = Seq.empty
)

/** Simple factory for construcing a [[Settings]] from arguments.
*
* This method primarily exists as a way to make future refactors that add
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/chisel3/simulator/SimulatorAPI.scala
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ trait SimulatorAPI {
timeout: Int,
chiselOpts: Array[String] = Array.empty,
firtoolOpts: Array[String] = Array.empty,
settings: Settings[T] = Settings.defaultRaw[T],
settings: Settings[T] = Settings.defaultTest[T],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Behavior change here too

Copy link
Contributor

Choose a reason for hiding this comment

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

This I missed, what's the change in behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 😄

additionalResetCycles: Int = 0,
subdirectory: Option[String] = None
)(
Expand Down
160 changes: 160 additions & 0 deletions src/test/scala-2/chiselTests/simulator/scalatest/ChiselSimSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,166 @@ class ChiselSimSpec extends AnyFunSpec with Matchers with ChiselSim with FileChe
)
}

it("should set macros to use init for Settings.defaultTest") {
class Foo extends SimulationTestHarness {
val (_, wrap) = Counter(true.B, 4)
done :<= wrap
success :<= true.B
}
simulateTest(new Foo, timeout = 100, settings = Settings.defaultTest[Foo])
io.Source
.fromFile(
FileSystems
.getDefault()
.getPath(implicitly[HasTestingDirectory].getDirectory.toString, "workdir-verilator", "Makefile")
.toFile
)
.mkString
.fileCheck()(
Comment on lines +157 to +165
Copy link
Contributor

Choose a reason for hiding this comment

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

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:

Suggested change
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()(

Copy link
Contributor

Choose a reason for hiding this comment

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

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 👀

"""|CHECK: '+define+ASSERT_VERBOSE_COND=!svsimTestbench.init'
|CHECK-NEXT: '+define+PRINTF_COND=!svsimTestbench.init'
|CHECK-NEXT: '+define+STOP_COND=!svsimTestbench.init'
|""".stripMargin
)
}

it("should set macros for SimulationTestHarnessInterface with separate IO") {
class SeparateIOHarness extends RawModule with SimulationTestHarnessInterface {
val clock = IO(Input(Clock()))
val init = IO(Input(Bool()))
val done = IO(Output(Bool()))
val success = IO(Output(Bool()))

withClockAndReset(clock, init) {
val (_, wrap) = Counter(true.B, 4)
done := wrap
success := true.B
}
}
simulateTest(new SeparateIOHarness, timeout = 100, settings = Settings.defaultTest[SeparateIOHarness])
io.Source
.fromFile(
FileSystems
.getDefault()
.getPath(implicitly[HasTestingDirectory].getDirectory.toString, "workdir-verilator", "Makefile")
.toFile
)
.mkString
.fileCheck()(
"""|CHECK: '+define+ASSERT_VERBOSE_COND=!svsimTestbench.init'
|CHECK-NEXT: '+define+PRINTF_COND=!svsimTestbench.init'
|CHECK-NEXT: '+define+STOP_COND=!svsimTestbench.init'
|""".stripMargin
)
}

it("should set macros for SimulationTestHarnessInterface with FlatIO") {
class FlatIOHarness extends RawModule with SimulationTestHarnessInterface {
private val io = FlatIO(new Bundle {
val clock = Input(Clock())
val init = Input(Bool())
val done = Output(Bool())
val success = Output(Bool())
})
def clock = io.clock
def init = io.init
def done = io.done
def success = io.success

withClockAndReset(clock, init) {
val (_, wrap) = Counter(true.B, 4)
io.done := wrap
io.success := true.B
}
}
simulateTest(new FlatIOHarness, timeout = 100, settings = Settings.defaultTest[FlatIOHarness])
io.Source
.fromFile(
FileSystems
.getDefault()
.getPath(implicitly[HasTestingDirectory].getDirectory.toString, "workdir-verilator", "Makefile")
.toFile
)
.mkString
.fileCheck()(
"""|CHECK: '+define+ASSERT_VERBOSE_COND=!svsimTestbench.init'
|CHECK-NEXT: '+define+PRINTF_COND=!svsimTestbench.init'
|CHECK-NEXT: '+define+STOP_COND=!svsimTestbench.init'
|""".stripMargin
)
}

it("should set macros for SimulationTestHarnessInterface with IO of a bundle") {
class BundleIOHarness extends RawModule with SimulationTestHarnessInterface {
private val io = IO(new Bundle {
val clock = Input(Clock())
val init = Input(Bool())
val done = Output(Bool())
val success = Output(Bool())
})
def clock = io.clock
def init = io.init
def done = io.done
def success = io.success

withClockAndReset(clock, init) {
val (_, wrap) = Counter(true.B, 4)
io.done := wrap
io.success := true.B
}
}
simulateTest(new BundleIOHarness, timeout = 100, settings = Settings.defaultTest[BundleIOHarness])
io.Source
.fromFile(
FileSystems
.getDefault()
.getPath(implicitly[HasTestingDirectory].getDirectory.toString, "workdir-verilator", "Makefile")
.toFile
)
.mkString
.fileCheck()(
"""|CHECK: '+define+ASSERT_VERBOSE_COND=!svsimTestbench.io_init'
|CHECK-NEXT: '+define+PRINTF_COND=!svsimTestbench.io_init'
|CHECK-NEXT: '+define+STOP_COND=!svsimTestbench.io_init'
|""".stripMargin
)
}

it("should set macros for SimulationTestHarnessInterface with renamed ports") {
class RenamedPortHarness extends RawModule with SimulationTestHarnessInterface {
val clk = IO(Input(Clock()))
val reset_n = IO(Input(Bool()))
val finished = IO(Output(Bool()))
val passed = IO(Output(Bool()))

def clock = clk
def init = reset_n
def done = finished
def success = passed

withClockAndReset(clk, reset_n) {
val (_, wrap) = Counter(true.B, 4)
finished := wrap
passed := true.B
}
}
simulateTest(new RenamedPortHarness, timeout = 100, settings = Settings.defaultTest[RenamedPortHarness])
io.Source
.fromFile(
FileSystems
.getDefault()
.getPath(implicitly[HasTestingDirectory].getDirectory.toString, "workdir-verilator", "Makefile")
.toFile
)
.mkString
.fileCheck()(
"""|CHECK: '+define+ASSERT_VERBOSE_COND=!svsimTestbench.reset_n'
|CHECK-NEXT: '+define+PRINTF_COND=!svsimTestbench.reset_n'
|CHECK-NEXT: '+define+STOP_COND=!svsimTestbench.reset_n'
|""".stripMargin
)
}

it("should allow for a user to customize the build directory") {
class Foo extends Module {
stop()
Expand Down