Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compilation Error when assigning from other clock #696

Open
da-steve101 opened this issue Apr 25, 2016 · 4 comments
Open

Compilation Error when assigning from other clock #696

da-steve101 opened this issue Apr 25, 2016 · 4 comments

Comments

@da-steve101
Copy link
Contributor

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
Copy link
Contributor

sdtwigg commented Apr 27, 2016

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
Copy link
Contributor

ucbjrl commented Apr 27, 2016

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

@sdtwigg
Copy link
Contributor

sdtwigg commented Apr 27, 2016

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
Copy link
Contributor Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants