From 89ef4d78e8f44f31df6530a6a4dee20d0ad0399f Mon Sep 17 00:00:00 2001 From: Chick Markley Date: Mon, 13 May 2019 18:08:25 -0700 Subject: RawModule with no reset should be able to use withClock method. (#1065) * RawModule with no reset should be able to use withClock method. - refactor ClockAndReset - now has `clockOpt: Option[Clock]` and `resetOpt: Option[Reset]` constructor params - convenience methods clock and reset tries to deref the option - ClockAndReset.empty is factory method for (None, None) - In Builder - forcedClock does not check resetOpt now - forcedReset does not check clockOpt now - withClock no longer looks at resetOpt - withReset no longer looks at clockOpt - Module starts with empty ClockAndReset * RawModule with no reset should be able to use withClock method. Refactor again based on @ducky64 comments - refactor away ClockAndReset, now builder just has a - currentClock - currentReset - withClock, withRest, withClockAndReset just use these fields directly * RawModule with no reset should be able to use withClock method. - Fixed typo in withReset handler, now picks up new reset --- .../src/main/scala/chisel3/core/Module.scala | 10 +++--- .../src/main/scala/chisel3/core/MultiClock.scala | 42 +++++++++++++++------- .../src/main/scala/chisel3/core/RawModule.scala | 3 +- .../src/main/scala/chisel3/internal/Builder.scala | 26 +++++++++----- src/test/scala/chiselTests/Clock.scala | 21 ++++++++--- 5 files changed, 71 insertions(+), 31 deletions(-) diff --git a/chiselFrontend/src/main/scala/chisel3/core/Module.scala b/chiselFrontend/src/main/scala/chisel3/core/Module.scala index f97d51ac..5cd48a6a 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Module.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Module.scala @@ -41,8 +41,9 @@ object Module extends SourceInfoDoc { val whenDepth: Int = Builder.whenDepth // Save then clear clock and reset to prevent leaking scope, must be set again in the Module - val clockAndReset: Option[ClockAndReset] = Builder.currentClockAndReset - Builder.currentClockAndReset = None + val (saveClock, saveReset) = (Builder.currentClock, Builder.currentReset) + Builder.currentClock = None + Builder.currentReset = None // Execute the module, this has the following side effects: // - set currentModule @@ -59,9 +60,10 @@ object Module extends SourceInfoDoc { "This is probably due to rewrapping a Module instance with Module()." + sourceInfo.makeMessage(" See " + _)) } - Builder.currentModule = parent // Back to parent! + Builder.currentModule = parent // Back to parent! Builder.whenDepth = whenDepth - Builder.currentClockAndReset = clockAndReset // Back to clock and reset scope + Builder.currentClock = saveClock // Back to clock and reset scope + Builder.currentReset = saveReset val component = module.generateComponent() Builder.components += component diff --git a/chiselFrontend/src/main/scala/chisel3/core/MultiClock.scala b/chiselFrontend/src/main/scala/chisel3/core/MultiClock.scala index 6af4da41..aaa03d78 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/MultiClock.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/MultiClock.scala @@ -2,14 +2,9 @@ package chisel3.core -import scala.language.experimental.macros - import chisel3.internal._ -import chisel3.internal.Builder.pushCommand -import chisel3.internal.firrtl._ -import chisel3.internal.sourceinfo.{SourceInfo} -private[chisel3] final case class ClockAndReset(clock: Clock, reset: Reset) +import scala.language.experimental.macros object withClockAndReset { // scalastyle:ignore object.name /** Creates a new Clock and Reset scope @@ -21,11 +16,17 @@ object withClockAndReset { // scalastyle:ignore object.name */ def apply[T](clock: Clock, reset: Reset)(block: => T): T = { // Save parentScope - val parentScope = Builder.currentClockAndReset - Builder.currentClockAndReset = Some(ClockAndReset(clock, reset)) + val parentClock = Builder.currentClock + val parentReset = Builder.currentReset + + Builder.currentClock = Some(clock) + Builder.currentReset = Some(reset) + val res = block // execute block + // Return to old scope - Builder.currentClockAndReset = parentScope + Builder.currentClock = parentClock + Builder.currentReset = parentReset res } } @@ -37,8 +38,15 @@ object withClock { // scalastyle:ignore object.name * @param block the block of code to run with new implicit Clock * @return the result of the block */ - def apply[T](clock: Clock)(block: => T): T = - withClockAndReset(clock, Module.reset)(block) + def apply[T](clock: Clock)(block: => T): T = { + // Save parentScope + val parentClock = Builder.currentClock + Builder.currentClock = Some(clock) + val res = block // execute block + // Return to old scope + Builder.currentClock = parentClock + res + } } object withReset { // scalastyle:ignore object.name @@ -48,7 +56,15 @@ object withReset { // scalastyle:ignore object.name * @param block the block of code to run with new implicit Reset * @return the result of the block */ - def apply[T](reset: Reset)(block: => T): T = - withClockAndReset(Module.clock, reset)(block) + def apply[T](reset: Reset)(block: => T): T = { + // Save parentScope + val parentReset = Builder.currentReset + Builder.currentReset = Some(reset) + val res = block // execute block + // Return to old scope + Builder.currentReset = parentReset + res + } + } diff --git a/chiselFrontend/src/main/scala/chisel3/core/RawModule.scala b/chiselFrontend/src/main/scala/chisel3/core/RawModule.scala index 397debcb..b224d9a3 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/RawModule.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/RawModule.scala @@ -146,7 +146,8 @@ abstract class MultiIOModule(implicit moduleCompileOptions: CompileOptions) val reset: Reset = IO(Input(Bool())) // Setup ClockAndReset - Builder.currentClockAndReset = Some(ClockAndReset(clock, reset)) + Builder.currentClock = Some(clock) + Builder.currentReset = Some(reset) private[core] override def initializeInParent(parentCompileOptions: CompileOptions): Unit = { implicit val sourceInfo = UnlocatableSourceInfo diff --git a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala index 1163171c..d825f39d 100644 --- a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala +++ b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala @@ -183,11 +183,13 @@ private[chisel3] class DynamicContext() { // Used to distinguish between no Module() wrapping, multiple wrappings, and rewrapping var readyForModuleConstr: Boolean = false var whenDepth: Int = 0 // Depth of when nesting - var currentClockAndReset: Option[ClockAndReset] = None + var currentClock: Option[Clock] = None + var currentReset: Option[Reset] = None val errors = new ErrorLog val namingStack = new internal.naming.NamingStack } +//scalastyle:off number.of.methods private[chisel3] object Builder { // All global mutable state must be referenced via dynamicContextVar!! private val dynamicContextVar = new DynamicVariable[Option[DynamicContext]](None) @@ -248,16 +250,22 @@ private[chisel3] object Builder { def whenDepth_=(target: Int): Unit = { dynamicContext.whenDepth = target } - def currentClockAndReset: Option[ClockAndReset] = dynamicContext.currentClockAndReset - def currentClockAndReset_=(target: Option[ClockAndReset]): Unit = { - dynamicContext.currentClockAndReset = target + def currentClock: Option[Clock] = dynamicContext.currentClock + def currentClock_=(newClock: Option[Clock]): Unit = { + dynamicContext.currentClock = newClock } - def forcedClockAndReset: ClockAndReset = currentClockAndReset match { - case Some(clockAndReset) => clockAndReset - case None => throwException("Error: No implicit clock and reset.") + + def currentReset: Option[Reset] = dynamicContext.currentReset + def currentReset_=(newReset: Option[Reset]): Unit = { + dynamicContext.currentReset = newReset } - def forcedClock: Clock = forcedClockAndReset.clock - def forcedReset: Reset = forcedClockAndReset.reset + + def forcedClock: Clock = currentClock.getOrElse( + throwException("Error: No implicit clock.") + ) + def forcedReset: Reset = currentReset.getOrElse( + throwException("Error: No implicit reset.") + ) // TODO(twigg): Ideally, binding checks and new bindings would all occur here // However, rest of frontend can't support this yet. diff --git a/src/test/scala/chiselTests/Clock.scala b/src/test/scala/chiselTests/Clock.scala index 78d60ed2..f508bb81 100644 --- a/src/test/scala/chiselTests/Clock.scala +++ b/src/test/scala/chiselTests/Clock.scala @@ -2,21 +2,34 @@ package chiselTests -import org.scalatest._ -import org.scalatest.prop._ - import chisel3._ +import chisel3.experimental.RawModule import chisel3.testers.BasicTester -import chisel3.util._ class ClockAsUIntTester extends BasicTester { assert(true.B.asClock.asUInt === 1.U) 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 = Driver.emit { () => new WithClockAndNoReset } + circuit.contains("reg a : UInt<1>, clock2") should be (true) + } } -- cgit v1.2.3