From 296fed733d56c47325e7474e4a8db650471bff7c Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Sat, 27 Jan 2024 17:55:07 +0000 Subject: [PATCH] Provide ImplicitClock and ImplicitReset (#3714) (#3769) Module's logic that creates the implicit clock and implicit reset within the Module's body is now implemented in new traits that users are free to mix in to RawModule. There are also now virtual methods implicitClock and implicitReset that the user can override to change what clock or reset is used. (cherry picked from commit abc96e20f9f1674e7d3c0b198b3c7960d1b83260) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Module.scala | 74 +++++++++++++++-- core/src/main/scala/chisel3/MultiClock.scala | 16 ++-- .../main/scala/chisel3/internal/Builder.scala | 43 ++++++++-- .../main/scala/chisel3/internal/package.scala | 12 +++ docs/src/cookbooks/cookbook.md | 46 +++++++++++ src/test/scala/chiselTests/Clock.scala | 36 -------- src/test/scala/chiselTests/ClockSpec.scala | 82 +++++++++++++++++++ src/test/scala/chiselTests/ResetSpec.scala | 44 ++++++++++ 8 files changed, 292 insertions(+), 61 deletions(-) delete mode 100644 src/test/scala/chiselTests/Clock.scala create mode 100644 src/test/scala/chiselTests/ClockSpec.scala diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 400f85de114..29b00629d0c 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -74,7 +74,7 @@ object Module extends SourceInfoDoc { // Save then clear clock and reset to prevent leaking scope, must be set again in the Module // Note that Disable is a function of whatever the current reset is, so it does not need a port // and thus does not change when we cross module boundaries - val (saveClock, saveReset) = (Builder.currentClock, Builder.currentReset) + val (saveClock, saveReset) = (Builder.currentClockDelayed, Builder.currentResetDelayed) val savePrefix = Builder.getPrefix Builder.clearPrefix() Builder.currentClock = None @@ -227,18 +227,20 @@ object Module extends SourceInfoDoc { * * @note Module instantiations must be wrapped in a Module() call. */ -abstract class Module extends RawModule { +abstract class Module extends RawModule with ImplicitClock with ImplicitReset { /** Override this to explicitly set the type of reset you want on this module , before any reset inference */ def resetType: Module.ResetType.Type = Module.ResetType.Default // Implicit clock and reset pins - final val clock: Clock = IO(Input(Clock()))(UnlocatableSourceInfo).suggestName("clock") - final val reset: Reset = IO(Input(mkReset))(UnlocatableSourceInfo).suggestName("reset") + final val clock: Clock = IO(Input(Clock()))(this._sourceInfo).suggestName("clock") + final val reset: Reset = IO(Input(mkReset))(this._sourceInfo).suggestName("reset") // TODO add a way to memoize hasBeenReset iff it is used - // TODO It's hard to remove these deprecated override methods because they're used by - // Chisel.QueueCompatibility which extends chisel3.Queue which extends chisel3.Module + override protected def implicitClock: Clock = clock + override protected def implicitReset: Reset = reset + + // TODO Delete these private var _override_clock: Option[Clock] = None private var _override_reset: Option[Bool] = None @deprecated("Use withClock at Module instantiation", "Chisel 3.5") @@ -253,6 +255,7 @@ abstract class Module extends RawModule { protected def override_reset_=(rhs: Option[Bool]): Unit = { _override_reset = rhs } + // End TODO Delete private[chisel3] def mkReset: Reset = { // Top module and compatibility mode use Bool for reset @@ -268,9 +271,6 @@ abstract class Module extends RawModule { } } - // Setup ClockAndReset - Builder.currentClock = Some(clock) - Builder.currentReset = Some(reset) // Note that we do no such setup for disable, it will default to hasBeenReset of the currentReset Builder.clearPrefix() @@ -283,6 +283,62 @@ abstract class Module extends RawModule { } } +/** Provides an implicit Clock for use _within_ the [[RawModule]] + * + * Be careful to define the Clock value before trying to use it. + * Due to Scala initialization order, the actual val defining the Clock must occur before any + * uses of the implicit Clock. + * + * @example + * {{{ + * class MyModule extends RawModule with ImplicitClock { + * // Define a Clock value, it need not be called "implicitClock" + * val clk = IO(Input(Clock())) + * // Implement the virtual method to tell Chisel about this Clock value + * // Note that though this is a def, the actual Clock is assigned to a val (clk above) + * override protected def implicitClock = clk + * // Now we have a Clock to use in this RawModule + * val reg = Reg(UInt(8.W)) + * } + * }}} + */ +trait ImplicitClock { self: RawModule => + + /** Method that should point to the user-defined Clock */ + protected def implicitClock: Clock + + Builder.currentClock = Some(Delayed(implicitClock)) +} + +/** Provides an implicit Reset for use _within_ the [[RawModule]] + * + * Be careful to define the Reset value before trying to use it. + * Due to Scala initialization order, the actual val defining the Reset object must occur before any + * uses of the implicit Reset. + * + * @example + * {{{ + * class MyModule extends RawModule with ImplicitReset { + * // Define a Reset value, it need not be called "implicitReset" + * val rst = IO(Input(AsyncReset())) + * // Implement the virtual method to tell Chisel about this Reset value + * // Note that though this is a def, the actual Reset is assigned to a val (rst above) + * override protected def implicitReset = clk + * // Now we have a Reset to use in this RawModule + * // Registers also require a clock + * val clock = IO(Input(Clock())) + * val reg = withClock(clock)(RegInit(0.U)) // Combine with ImplicitClock to get rid of this withClock + * } + * }}} + */ +trait ImplicitReset { self: RawModule => + + /** Method that should point to the user-defined Reset */ + protected def implicitReset: Reset + + Builder.currentReset = Some(Delayed(implicitReset)) +} + package internal { object BaseModule { diff --git a/core/src/main/scala/chisel3/MultiClock.scala b/core/src/main/scala/chisel3/MultiClock.scala index 659cf03a082..7561d819635 100644 --- a/core/src/main/scala/chisel3/MultiClock.scala +++ b/core/src/main/scala/chisel3/MultiClock.scala @@ -26,11 +26,11 @@ object withClockAndReset { */ def apply[T](clock: Option[Clock], reset: Option[Reset])(block: => T): T = { // Save parentScope - val parentClock = Builder.currentClock - val parentReset = Builder.currentReset + val parentClock = Builder.currentClockDelayed + val parentReset = Builder.currentResetDelayed - Builder.currentClock = clock - Builder.currentReset = reset + Builder.currentClock = clock.map(Delayed(_)) + Builder.currentReset = reset.map(Delayed(_)) val res = block // execute block @@ -59,8 +59,8 @@ object withClock { */ def apply[T](clock: Option[Clock])(block: => T): T = { // Save parentScope - val parentClock = Builder.currentClock - Builder.currentClock = clock + val parentClock = Builder.currentClockDelayed + Builder.currentClock = clock.map(Delayed(_)) val res = block // execute block // Return to old scope Builder.currentClock = parentClock @@ -86,8 +86,8 @@ object withReset { */ def apply[T](reset: Option[Reset])(block: => T): T = { // Save parentScope - val parentReset = Builder.currentReset - Builder.currentReset = reset + val parentReset = Builder.currentResetDelayed + Builder.currentReset = reset.map(Delayed(_)) val res = block // execute block // Return to old scope Builder.currentReset = parentReset diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index e05bff3f0f2..2e31c82bb7d 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -25,6 +25,7 @@ import scala.collection.mutable import scala.annotation.tailrec import java.io.File import scala.util.control.NonFatal +import chisel3.ChiselException private[chisel3] class Namespace(keywords: Set[String], separator: Char = '_') { // This HashMap is compressed, not every name in the namespace is present here. @@ -523,10 +524,12 @@ private[chisel3] class DynamicContext( // Used to distinguish between no Module() wrapping, multiple wrappings, and rewrapping var readyForModuleConstr: Boolean = false var whenStack: List[WhenContext] = Nil - var currentClock: Option[Clock] = None - var currentReset: Option[Reset] = None - var currentDisable: Disable.Type = Disable.BeforeReset - var layerStack: List[layer.Layer] = layer.Layer.root :: Nil + // Clock and Reset are "Delayed" because ImplicitClock and ImplicitReset need to set these values, + // But the clock or reset defined by the user won't yet be initialized + var currentClock: Option[Delayed[Clock]] = None + var currentReset: Option[Delayed[Reset]] = None + var currentDisable: Disable.Type = Disable.BeforeReset + var layerStack: List[layer.Layer] = layer.Layer.root :: Nil val errors = new ErrorLog(warningFilters, sourceRoots, throwOnFirstError) val namingStack = new NamingStack @@ -791,13 +794,35 @@ private[chisel3] object Builder extends LazyLogging { def currentWhen: Option[WhenContext] = dynamicContext.whenStack.headOption - def currentClock: Option[Clock] = dynamicContext.currentClock - def currentClock_=(newClock: Option[Clock]): Unit = { + // Helper for reasonable errors when clock or reset value not yet initialized + private def getDelayed[A](field: String, dc: Delayed[A]): A = { + val result = dc.value + if (result == null) { + // TODO add SourceInfo and change to Builder.exception + throwException( + s"The implicit $field is null which means the code that sets its definition has not yet executed." + ) + } + result + } + + /** Safely get the current Clock for use */ + def currentClock: Option[Clock] = + dynamicContext.currentClock.map(d => getDelayed("clock", d)) + + /** Get the underlying box around current Clock, only used for saving the value */ + def currentClockDelayed: Option[Delayed[Clock]] = dynamicContext.currentClock + def currentClock_=(newClock: Option[Delayed[Clock]]): Unit = { dynamicContext.currentClock = newClock } - def currentReset: Option[Reset] = dynamicContext.currentReset - def currentReset_=(newReset: Option[Reset]): Unit = { + /** Safely get the current Reset for use */ + def currentReset: Option[Reset] = + dynamicContext.currentReset.map(d => getDelayed("reset", d)) + + /** Get the underlying box around current Reset, only used for saving the value */ + def currentResetDelayed: Option[Delayed[Reset]] = dynamicContext.currentReset + def currentReset_=(newReset: Option[Delayed[Reset]]): Unit = { dynamicContext.currentReset = newReset } @@ -818,9 +843,11 @@ private[chisel3] object Builder extends LazyLogging { } def forcedClock: Clock = currentClock.getOrElse( + // TODO add implicit clock change to Builder.exception throwException("Error: No implicit clock.") ) def forcedReset: Reset = currentReset.getOrElse( + // TODO add implicit clock change to Builder.exception throwException("Error: No implicit reset.") ) diff --git a/core/src/main/scala/chisel3/internal/package.scala b/core/src/main/scala/chisel3/internal/package.scala index 70ee952bfbb..308b5628420 100644 --- a/core/src/main/scala/chisel3/internal/package.scala +++ b/core/src/main/scala/chisel3/internal/package.scala @@ -11,6 +11,7 @@ import chisel3.internal.Builder.Prefix import scala.util.Try import scala.annotation.implicitNotFound import scala.collection.mutable +import chisel3.ChiselException package object internal { @@ -143,6 +144,17 @@ package object internal { } } + /** This is effectively a "LazyVal" box type, we can create the object but delay executing the argument + * + * @note This is similar to cats.Eval.later but we don't depend on Cats + */ + private[chisel3] class Delayed[A](a: => A) { + lazy val value: A = a + } + private[chisel3] object Delayed { + def apply[A](a: => A): Delayed[A] = new Delayed(a) + } + /** The list of banned type alias words which will cause generation of bad FIRRTL. These are usually * keyword tokens that would be automatically lexed by firtool, and so cause parsing errors. */ diff --git a/docs/src/cookbooks/cookbook.md b/docs/src/cookbooks/cookbook.md index 4294112353e..ec03a334fd2 100644 --- a/docs/src/cookbooks/cookbook.md +++ b/docs/src/cookbooks/cookbook.md @@ -29,6 +29,7 @@ Please note that these examples make use of [Chisel's scala-style printing](../e * [How do I do subword assignment (assign to some bits in a UInt)?](#how-do-i-do-subword-assignment-assign-to-some-bits-in-a-uint) * [How do I create an optional I/O?](#how-do-i-create-an-optional-io) * [How do I create I/O without a prefix?](#how-do-i-create-io-without-a-prefix) +* [How do I override the implicit clock or reset within a Module?](#how-do-i-override-the-implicit-clock-or-reset-within-a-module) * [How do I minimize the number of bits used in an output vector](#how-do-i-minimize-the-number-of-bits-used-in-an-output-vector) * [How do I resolve "Dynamic index ... is too wide/narrow for extractee ..."?](#dynamic-index-too-wide-narrow) * Predictable Naming @@ -746,6 +747,51 @@ Note that `io_` is nowhere to be seen! getVerilogString(new MyModule) ``` +### How do I override the implicit clock or reset within a Module? + +To change the clock or reset for a region of code, use `withClock`, `withReset`, or `withClockAndReset`. +See [Multiple Clock Domains](../explanations/multi-clock) for examples and details. + +To override the clock or reset for the entire scope of the `Module`, you can mixin the `ImplicitClock` and `ImplicitReset` traits. + +For example, you could "gate" the default implicit clock as follows: + +```scala mdoc:silent:reset +import chisel3._ +class MyModule extends Module with ImplicitClock { + val gate = IO(Input(Bool())) + val in = IO(Input(UInt(8.W))) + val out = IO(Output(UInt(8.W))) + // We could just assign this to val implicitClock, but this allows us to give it a custom name + val gatedClock = (clock.asBool || gate).asClock + // The trait requires us to implement this method referring to the clock + // Note that this is a def, but the actual clock value must be assigned to a val + override protected def implicitClock = gatedClock + + val r = Reg(UInt(8.W)) + out := r + r := in +} +``` + +This gives the following Verilog: + +```scala mdoc:verilog +def func(): String = { + // This example uses a Reg to we need to disable randomization + val prettyArgs = Array("--disable-all-randomization", "--strip-debug-info") + circt.stage.ChiselStage.emitSystemVerilog(new MyModule, firtoolOpts = prettyArgs) +} +func() +``` + +If you do not care about the name of the overriden clock, you can just assign it to `val implicitClock`: +```scala +override protected val implicitClock = (clock.asBool || gate).asClock +``` + +`ImplicitReset` works analogously to `ImplicitClock`. + ### How do I minimize the number of bits used in an output vector? Use inferred width and a `Seq` instead of a `Vec`: diff --git a/src/test/scala/chiselTests/Clock.scala b/src/test/scala/chiselTests/Clock.scala deleted file mode 100644 index 7ad7083ce77..00000000000 --- a/src/test/scala/chiselTests/Clock.scala +++ /dev/null @@ -1,36 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -package chiselTests - -import chisel3._ -import chisel3.testers.BasicTester -import circt.stage.ChiselStage - -class ClockAsUIntTester extends BasicTester { - assert(true.B.asClock.asUInt === 1.U) - assert(true.B.asClock.asBool === true.B) - stop() -} - -class WithClockAndNoReset extends RawModule { - val clock1 = IO(Input(Clock())) - val clock2 = IO(Input(Clock())) - val in = IO(Input(Bool())) - val out = IO(Output(Bool())) - val a = withClock(clock2) { - RegNext(in) - } - - out := a -} - -class ClockSpec extends ChiselPropSpec { - property("Bool.asClock.asUInt should pass a signal through unaltered") { - assertTesterPasses { new ClockAsUIntTester } - } - - property("Should be able to use withClock in a module with no reset") { - val circuit = ChiselStage.emitCHIRRTL(new WithClockAndNoReset) - circuit.contains("reg a : UInt<1>, clock2") should be(true) - } -} diff --git a/src/test/scala/chiselTests/ClockSpec.scala b/src/test/scala/chiselTests/ClockSpec.scala new file mode 100644 index 00000000000..a4494aa722b --- /dev/null +++ b/src/test/scala/chiselTests/ClockSpec.scala @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: Apache-2.0 + +package chiselTests + +import chisel3._ +import chisel3.testers.BasicTester +import circt.stage.ChiselStage + +class ClockAsUIntTester extends BasicTester { + assert(true.B.asClock.asUInt === 1.U) + assert(true.B.asClock.asBool === true.B) + stop() +} + +class WithClockAndNoReset extends RawModule { + val clock1 = IO(Input(Clock())) + val clock2 = IO(Input(Clock())) + val in = IO(Input(Bool())) + val out = IO(Output(Bool())) + val a = withClock(clock2) { + RegNext(in) + } + + out := a +} + +class ClockSpec extends ChiselPropSpec { + property("Bool.asClock.asUInt should pass a signal through unaltered") { + assertTesterPasses { new ClockAsUIntTester } + } + + property("Should be able to use withClock in a module with no reset") { + val circuit = ChiselStage.emitCHIRRTL(new WithClockAndNoReset) + circuit.contains("reg a : UInt<1>, clock2") should be(true) + } + + property("Should be able to override the value of the implicit clock") { + val verilog = ChiselStage.emitSystemVerilog(new Module { + val gate = IO(Input(Bool())) + val in = IO(Input(UInt(8.W))) + val out = IO(Output(UInt(8.W))) + val gatedClock = (clock.asBool || gate).asClock + override protected def implicitClock = gatedClock + + val r = Reg(UInt(8.W)) + out := r + r := in + }) + verilog should include("gatedClock = clock | gate;") + verilog should include("always @(posedge gatedClock)") + } + + property("Should be able to add an implicit clock to a RawModule") { + val verilog = ChiselStage.emitSystemVerilog(new RawModule with ImplicitClock { + val foo = IO(Input(Bool())) + val in = IO(Input(UInt(8.W))) + val out = IO(Output(UInt(8.W))) + override protected val implicitClock = (!foo).asClock + + val r = Reg(UInt(8.W)) + out := r + r := in + }) + verilog should include("always @(posedge implicitClock)") + } + + property("Chisel should give a decent error message if you try to use a clock before defining it") { + val e = the[ChiselException] thrownBy ( + ChiselStage.emitCHIRRTL( + new RawModule with ImplicitClock { + val r = Reg(UInt(8.W)) + val foo = IO(Input(Clock())) + override protected def implicitClock = foo + }, + args = Array("--throw-on-first-error") + ) + ) + e.getMessage should include( + "The implicit clock is null which means the code that sets its definition has not yet executed." + ) + } +} diff --git a/src/test/scala/chiselTests/ResetSpec.scala b/src/test/scala/chiselTests/ResetSpec.scala index 315871540a5..5d7e1a0ce14 100644 --- a/src/test/scala/chiselTests/ResetSpec.scala +++ b/src/test/scala/chiselTests/ResetSpec.scala @@ -103,6 +103,34 @@ class ResetSpec extends ChiselFlatSpec with Utils { fir should include("input reset : AsyncReset") } + they should "be able to override the value of the implicit reset" in { + val verilog = ChiselStage.emitSystemVerilog(new Module { + val gate = IO(Input(Bool())) + val in = IO(Input(UInt(8.W))) + val out = IO(Output(UInt(8.W))) + override protected val implicitReset = reset.asBool || gate + val r = RegInit(0.U) + out := r + r := in + }) + verilog should include("if (reset | gate)") + } + + they should "be able to add an implicit clock to a RawModule" in { + val verilog = ChiselStage.emitSystemVerilog(new RawModule with ImplicitReset { + val foo = IO(Input(Bool())) + val in = IO(Input(UInt(8.W))) + val out = IO(Output(UInt(8.W))) + override protected val implicitReset = !foo + + val clk = IO(Input(Clock())) + val r = withClock(clk)(RegInit(0.U)) + out := r + r := in + }) + verilog should include("if (~foo)") + } + they should "be able to have parameterized top level reset type" in { class MyModule(hasAsyncNotSyncReset: Boolean) extends Module { override def resetType = if (hasAsyncNotSyncReset) Module.ResetType.Asynchronous else Module.ResetType.Synchronous @@ -125,4 +153,20 @@ class ResetSpec extends ChiselFlatSpec with Utils { }) } } + + it should "give a decent error message if you try to use a reset before defining it" in { + val e = the[ChiselException] thrownBy ( + ChiselStage.emitCHIRRTL( + new RawModule with ImplicitReset { + val r = Module.reset + val foo = IO(Input(AsyncReset())) + override protected def implicitReset = foo + }, + args = Array("--throw-on-first-error") + ) + ) + e.getMessage should include( + "The implicit reset is null which means the code that sets its definition has not yet executed." + ) + } }