Skip to content

Commit

Permalink
Fix Reg() to properly handle clocks as rvalues (#3775)
Browse files Browse the repository at this point in the history
Previously, it was not calling .ref. It was thus not properly:
1. Checking that the clock is bound hardware
2. Checking that the clock is visible in the current scope
3. Reifying the clock if its a view

Note that RegInit() was already correctly calling .ref.

(cherry picked from commit 1c348c5)

# Conflicts:
#	src/test/scala/chiselTests/ClockSpec.scala
  • Loading branch information
jackkoenig authored and mergify[bot] committed Jan 29, 2024
1 parent 449f74a commit a284022
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 1 deletion.
2 changes: 1 addition & 1 deletion core/src/main/scala/chisel3/Reg.scala
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ object Reg {
requireIsChiselType(t, "reg type")
}
val reg = if (!t.mustClone(prevId)) t else t.cloneTypeFull
val clock = Node(Builder.forcedClock)
val clock = Builder.forcedClock.ref

reg.bind(RegBinding(Builder.forcedUserModule, Builder.currentWhen))
pushCommand(DefReg(sourceInfo, reg, clock))
Expand Down
131 changes: 131 additions & 0 deletions src/test/scala/chiselTests/ClockSpec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// 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."
)
}

property("Chisel should give a decent error message if you use an unbound Clock") {
val e = the[ChiselException] thrownBy (
ChiselStage.emitCHIRRTL(
new RawModule {
withClock(Clock()) {
val r = Reg(UInt(8.W))
}
},
args = Array("--throw-on-first-error")
)
)
e.getMessage should include(
"'Clock' must be hardware, not a bare Chisel type. Perhaps you forgot to wrap it in Wire(_) or IO(_)?"
)
}

property("Chisel should give a decent error message if you use a Clock from another scope") {
val e = the[ChiselException] thrownBy (
ChiselStage.emitCHIRRTL(
new RawModule {
override def desiredName = "Parent"
val child = Module(new RawModule {
override def desiredName = "Child"
val clock = Wire(Clock())
})
withClock(child.clock) {
val r = Reg(UInt(8.W))
}
},
args = Array("--throw-on-first-error")
)
)
e.getMessage should include(
"operand 'Child.clock: Wire[Clock]' is not visible from the current module Parent"
)
}

property("Chisel should support Clocks from views") {
import chisel3.experimental.dataview._
val chirrtl = ChiselStage.emitCHIRRTL(new RawModule {
val clock = IO(Clock())
val view = clock.viewAs[Clock]
withClock(view) {
val r = Reg(UInt(8.W))
}
})
chirrtl should include("reg r : UInt<8>, clock")
}
}

0 comments on commit a284022

Please sign in to comment.