what happens for combinational loops through a register's async reset input?

[programmerjake]

in the section about combinational loops, it doesn't forbid combinational loops like so:

FIRRTL version 4.0.0
circuit Foo:
    module Foo:
        input d: UInt<1>
        input clk: Clock
        input do_reset: UInt<1>
        output y: UInt<1>
        wire rst: AsyncReset
        connect rst, asAsyncReset(UInt<1>(0))
        regreset r: UInt<1>, clk, rst, UInt<1>(0)
        when do_reset:
            connect rst, asAsyncReset(not(r)) ; not forbidden since the loop went through a register
        connect r, d
        connect y, r

Is this an oversight or expected?

[mmaloney-sf]

I just want to make sure: Your expectation is that an async reset should be considered a combinational input to a register. Correct?

If so, I think that's a reasonable assumption to have. It is also the kind of technicality that isn't properly covered by the specification today. The section on Combinational Loops makes no attempt to define what a combinational loop is. Asynchronous reset is a newer feature, so I imagine we could classify this squarely under 'oversight'.

[programmerjake]

I just want to make sure: Your expectation is that an async reset should be considered a combinational input to a register. Correct?

yes

If so, I think that's a reasonable assumption to have. It is also the kind of technicality that isn't properly covered by the specification today. The section on Combinational Loops makes no attempt to define what a combinational loop is. Asynchronous reset is a newer feature, so I imagine we could classify this squarely under 'oversight'.

ok, so then I think this should be added to the section on Combinational Loops.

[seldridge]

I agree that this seems like the kind of thing that should be disallowed. The description of a combinational loop should be updated to include language that specifically states that a feedback path from a register to its asynchronous reset port should be disallowed. (It seems like most places will consider this a "combinational loop" [1] without further explanation, though.)

[programmerjake]

what about feedback from a register/memory to its clock input(s)? seems similar enough...

[seldridge]

It seems like the same kind of problem.

One thing I will note is that we've been discussing adding a notion of "domains" to FIRRTL which should subsume a lot of this. This would then make some of these things illegal by construction, unless a user was using explicit operations to opt out of the safety (which could be legal). This has been primarily brought up as a way to reject circuits with illegal clock domain crossings while still having enough in FIRRTL to describe synchronizers which must violate naive clock domain crossing checks. (You can then extend this to reset domains, power domains, and other things that you'd like to leave to a user to define.)

One of my mild concerns with combinational loop checking, and additional restrictions like no paths from output to reset or clock port, is that it isn't defined in such a way that a user can choose to opt out or extend it. It's really just a way of saying, "I don't want these two things to be connected". However, that falls down for things like external modules. In the past, this has been solved with annotations on the external module that indicate combinational paths. This has always been unsatisfying as it relies on ad-hoc compiler extensions without anything built-in to the language.

Narrow combinational loop checking (a user didn't create an osciallator) always seemed fine (unless the user actually wants to create an oscillator).

Additionally, design verification engineers could do things like the OP or the register/memory to clock connection and it's probably fine if it's just DV code. (This is a hypothetical that I do not have data to back up.)

[tymcauley]

what about feedback from a register/memory to its clock input(s)? seems similar enough...

While it might not be the most common use-case for FIRRTL users, a feedback path from a register's output to its clock input is exactly how you construct a Click controller, which is an element in some asynchronous/self-timed circuits.

From a different perspective, if I were thinking with my static-timing-analysis hat, those timing tools usually break timing paths once you hit the clock input of a register, so a path from a register's output to its clock input would be legal there too.

I'd vote that we continue to allow register-output-to-clock-input paths, unless there seems to be a good justification for disallowing them.

One thing I will note is that we've been discussing adding a notion of "domains" to FIRRTL which should subsume a lot of this. This would then make some of these things illegal by construction, unless a user was using explicit operations to opt out of the safety (which could be legal). This has been primarily brought up as a way to reject circuits with illegal clock domain crossings while still having enough in FIRRTL to describe synchronizers which must violate naive clock domain crossing checks. (You can then extend this to reset domains, power domains, and other things that you'd like to leave to a user to define.)

This sounds awesome, I've always though that clock-/reset-domain-crossing analysis should be doable in CIRCT, but the specification of what counts as in-domain, and how you cross domains, seems tricky.

[seldridge]

Good point @tymcauley. This is the kind of thing that I worry about.

Even for combinational cycle detection, you can't express an inverter chain which is an entirely sane test circuit to tape out. It does go outside of FIRRTL (and Chisel's) normal application domain of digital, synchronous circuits. However, it's an entirely reasonable thing to write in Verilog and tape out, yet FIRRTL explicitly disallows it for the reasons that it is a common user error.

What I want out of the domain work is to make it such that a user could express an inverter chain in FIRRTL, have it error by default, but have the right FIRRTL primitives such that the user can write their FIRRTL such that the compiler knows that it is safe. (This kind of thing is required to write a synchronizer.)

This sounds awesome, I've always though that clock-/reset-domain-crossing analysis should be doable in CIRCT, but the specification of what counts as in-domain, and how you cross domains, seems tricky.

My hope is that this isn't an analysis, but first-class in FIRRTL. I.e., using MLIR syntax:

firrtl.connect %b, %a : (!firrtl.uint<1, domainA>, !firrtl.uint<2, domainB>) -> ()

The above can be outright rejected just from examining the types. You do then have to have the ability to do an unsafe cast from one domain to another to write a synchronizer.

[programmerjake]

well, there are valid use cases for the other types of combinational loops too, e.g.:

• making latches out of cross-coupled nand gates or nor gates
• making a counter with arbitrary modulus by wiring the counter's asynchronous reset input to a signal that goes high when the output is equal to the modulus, e.g. for a mod 3 counter set it to the and of both output bits. this is commonly used with jellybean counter chips such as the 4017. (yes that causes a glitch, the application note uses a latch connected to the clock and one of the decoded outputs to avoid a glitch on the reset pin)

[mmaloney-sf]

Even for combinational cycle detection, you can't express an inverter chain which is an entirely sane test circuit to tape out. It does go outside of FIRRTL (and Chisel's) normal application domain of digital, synchronous circuits. However, it's an entirely reasonable thing to write in Verilog and tape out, yet FIRRTL explicitly disallows it for the reasons that it is a common user error.

This is closely related to the standard point that type systems are always conservative: No matter how fancy your type system, there will always be some valid program that gets rejected.

But there are reasons the valid programs get rejected as well. In the case of a ring buffer, what you have may be a standard circuit. But it is not subject to timing discipline, and it breaks an implicit assumption that would hold for any other FIRRTL circuit: that holding the inputs constant for sufficiently long will lead to a stable value.

Similar to this mechanic are tristate buffers and latches. They are totally standard circuits. But it would be a stretch to say FIRRTL supports either.

If I wore the hat, I would take the more conservative route and forbid register Q-to-Reset and Q-to-Clock loops. For users who need them, intrinsics are meant to fill the role of any unsupported primitive. If demand is high enough, it gives us the most liberty to address it (eg, relaxing the loop rules, adding a new primitive, or perhaps, adding a notion of domain).

[programmerjake]

forbid register Q-to-Reset and Q-to-Clock loops

there's also memory read data to clock loops (both for the same memory port or a different write port)