chisel2-deprecated icon indicating copy to clipboard operation
chisel2-deprecated copied to clipboard

Compilation Error when assigning from other clock

Open da-steve101 opened this issue 9 years ago • 4 comments

I am getting an undeclared variable error for the following:

class UserMod extends Module {
  val io = new Bundle {
    val out = UInt( OUTPUT,8 )
  }
  io.out := UInt( 0 )
  val otherClk = Clock()
  io.out( 7, 0 ) := Reg( next = UInt( 0, 8 ), clock = otherClk )
}

chiselMain( Array("--backend", "c", "--genHarness", "--compile"), () => Module( new UserMod ) )

da-steve101 avatar Apr 25 '16 13:04 da-steve101

Try removing the (7,0) from io.out and the origing io.out assignment. The signal is 8 bits wide so the (7,0) should be superfluous (although glitches in the internal implementation for subword updates mean it is actually injecting logic). So just:

class UserMod extends Module {
  val io = new Bundle {
    val out = UInt( OUTPUT,8 )
  }
  val otherClk = Clock()
  io.out := Reg(next = UInt(0, width=8), clock = otherClk)
}

There is a separate issue you should be aware of that, essentially, chisel assumes all logic related to IOs lies in the implicit clock domain. This is related to why it ended up splitting the generated code into the functions for both clock domains.

Edit: updated the code to conform closer to my personal stylistic tastes

sdtwigg avatar Apr 27 '16 18:04 sdtwigg

Thanks Steven. That indeed fixes the compilation problem.

The manual states that there are two ways to send data between clock domains:

  • two-register synchronizer (for 1-bit signals)
  • asynchronous fifo

It sees to me we should at least warn about other attempts to connect signals from different clock domains.

On Apr 27, 2016, at 11:50 AM, Stephen Twigg [email protected] wrote:

Try removing the (7,0) from io.out and the origing io.out assignment. The signal is 8 bits wide so the (7,0) should be superfluous (although glitches in the internal implementation for subword updates mean it is actually injecting logic). So just:

class UserMod extends Module {

val io = new Bundle {

val out = UInt( OUTPUT,8 ) }

val otherClk = Clock () io.out := Reg(next = UInt( 0, 8 ), clock = otherClk ) }

There is a separate issue you should be aware of that, essentially, chisel assumes all logic related to IOs lies in the implicit clock domain. This is related to why it ended up splitting the generated code into the functions for both clock domains.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub

ucbjrl avatar Apr 27 '16 20:04 ucbjrl

I would go one step farther and say that you can only cross clock domains in Modules marked as permitting clock domain crossings. This saves us from having to special case the AsyncFIFO module and allows users to use their own. (This is particularly helpful since the Chisel.AsyncFIFO module abides by a specific reset contract that users may not want. It definitely still serves as a good example implementation for an AsyncFIFO.)

You would further ban implicit synchronizers (instead favoring an actual module e.g. called Synchronizer). This prevents people from accidentally crossing a clock domain or trying to implement a multi-bit synchronizer naively (which can lead to data falling out of sync across bits).

Then again, it may be worth punting on this issue (to chisel3). A clock domain check is particularly amenable to a FIRRTL pass and, further, it is a FIRRTL pass that users may want to customize. For example, if you know one clock is a derivative of another (e.g. from a clock divider) then you don't need as much clock crossing machinery.

sdtwigg avatar Apr 27 '16 21:04 sdtwigg

Hi, yeah I know the bit extract is redundant in this case. It was just a simplification of what I was actually trying to do. I am fine with leaving it for chisel 3 to fix, but in the meantime, perhaps a warning message with something along the lines of "assigning from two clock domains" rather than failing with a compilation error which is difficult to debug is merited?

da-steve101 avatar May 02 '16 02:05 da-steve101