ucb-bar / chisel2-deprecated

chisel.eecs.berkeley.edu
387 stars 89 forks source link

Compilation Error when assigning from other clock #696

Open da-steve101 opened 8 years ago

da-steve101 commented 8 years ago

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 ) )
sdtwigg commented 8 years ago

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

ucbjrl commented 8 years ago

Thanks Steven. That indeed fixes the compilation problem.

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

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 notifications@github.com 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

sdtwigg commented 8 years ago

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.

da-steve101 commented 8 years ago

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?