[RFC] Warning/Error if CDC not managed correctly

[Martoni]

Question/request asked first on Stackoverflow.

Type of issue: feature request
Impact: API modification
Development Phase: request

What is the current behavior?

Currently, it's possible to manage different Clock domains with Chisel3. But if we need to read/write a signal through two different clock domains it's important to manage metastability (with dual d-latch, asynchronous fifo, ...).

If we don't manage this cross-clock-domain Chisel will not warn or error about it.

What is the expected behavior?

If we don't says explicitly to Chisel that we managed this crossing. The behavior expected should be in minimum to warn, maybe to generate an error.

What is the use case for changing the behavior?

In the wiki example:

 class MultiClockModule extends Module {
  val io = IO(new Bundle {
    val clockB = Input(Clock())
    val resetB = Input(Bool())
    val stuff = Input(Bool())
  })

  // This register is clocked against the module clock.
  val regClock = RegNext(io.stuff)

  withClockAndReset (io.clockB, io.resetB) {
    // In this withClock scope, all synchronous elements are clocked against io.clockB.
    // Reset for flops in this domain is using the explicitly provided reset io.resetB.

    // This register is clocked against io.clockB.
    val regClockB = RegNext(io.stuff)
  }

  // This register is also clocked against the module clock.
  val regClock2 = RegNext(io.stuff)
}

We could add a function named cdc() for example to signal Chisel that we know that signal is crossing clock domain :
If we write this we will have an error/warning :

val regClockB = RegNext(io.stuff)

And to delete the error we could do :

val regClockB = cdc(RegNext(io.stuff))

With maybe adding the names of clocks domains implied in arguments ?

[jackkoenig]

Clock Domain Crossing checking is a badly needed feature, thank you for filing an issue. I have some thoughts on this:

One implementation possibility is to annotate legal CDCs and then check it in FIRRTL. There are certain disadvantages to this approach, since it puts correctness information in annotations instead of in the actual IR, but I think it's better than trying to represent complex structures as first-class citizens in the IR. There is a lingering PR against FIRRTL (chipsalliance/firrtl#937) that could make such a check fairly easy to implement. We discussed this PR in the last dev meeting; it needs some work but we decided that it's useful enough as is so we should probably go ahead and push it through.

This makes the check easy enough, but how do we annotate the legal CDCs? The most obvious answer is that we have normal utilities you're expected to use and annotate them there. People can obviously define their own CDC utilities in which case they'll merely have to use the annotation as well.

As suggested here (#1067) we should likely standardize common CDC primitives either externally of rocket-chip or perhaps in chisel3 itself. Trying to pull them into chisel3 itself might run into code ownership concerns, but I'm sure that can be resolved. Before trying to put them in chisel3 proper, I'd also like to seem them implemented natively to whatever extent possible (needs #1011).

Another potential issue is that clock gating is a common degenerate case of CDC. You don't have primitives between the registers, rather you know it's legal based on the relationship between the registers' clocks. The issue is that we don't natively support clock gates (see chipsalliance/firrtl#1017), so one could annotate every single register but that would be incredibly onerous. A better approach is probably to annotate the blackboxed clock muxes that implement clock gating and include this information in the FIRRTL pass that checks CDC legality. We can also consider the question posed in chipsalliance/firrtl#1017 and actually implement clock muxes natively, similarly to how memories are handled.

[jackkoenig]

I would add that our CDC library should be formally verified since it's so hard to get right. My default assumption would be to use https://github.com/YosysHQ/yosys, but there are other options as well. I think for maximal trust, we should be able to generate formal verification collateral for the emitted Verilog since that allows people to trust the Verilog they're using to tapeout.