calyx
calyx copied to clipboard
calyxir
Omit the @clk and @reset ports when inlining cell ports to the component signature
When incorporating the 'external' keyword syntax to the component signature(described in #992), I ran into the following error message when inlining cell ports to the component signature(4th task in tracker #998):
The solution turned out to be a simple one: ignore the ports marked with @clk and @reset when inlining the ports of the cell to the signature.
Through discussion with @rachitnigam and @sampsyo, we hypothesize that possible reasons could be: the clk_insertion pass automatically adds an assignment for the clk signal of every component; during the port inlining, a new port is created in the component that is not a @clk port but also gets a clk signal, conflicting with the original clk signal.
However, we still need careful thinking to say exactly why this is the right fix for the problem.
The problem that @paili0628 ran into happens because clk and reset signals are special: they are automatically threaded through the design into the primitives that define them using the clk-insertion and reset-insertion passes. When @paili0628's pass added those ports into the component signature, the component ended up passing multiple clk and reset signals into the primitives into the design.
There is a broader question about what to do with such "universally-threaded" signals in the design. What is the principle in deciding to omit them?
Yeah, this—defining a policy for when/how to insert this stuff—is a hard question. It reminds me, in a weird way, of having global state… nobody "owns" these signals and they can't be encapsulated. We could make this even harder by trying to think about multiple clocks, but even the single-clock version is confusing enough.
I don't know the right answer, but one possibly-unsatisfying policy is to somehow uphold an invariant that says that these insertion passes only ever run once on a given piece of code. Not sure how to do that. Or we could, like, make them implicit until the last minute—the signals don't even exist until we cross some kind of threshold in the lowering process. (On the assumption that "normal" Calyx programs and optimization passes don't want to deal with them.) All of this is kind of messy, though, and I'm lacking a perfect insight for how to clean it up.
So the weird thing is that the parser automatically adds all of these signals to a component's definition if and only if it is missing them. For example, a component that defines @clk and @reset ports will not get another @clk and @reset port. This implicit behavior has always been somewhat weird. The CIRCT frontend, for example, requires you to manually add all of these ports to all components and errors if you don't.
Ah yeah, right, implicitly inserting those during parsing is pretty funky. We could shift policies to say that they get inserted at the very end of lowering instead, and components don't need to have them before then? Really unsure about this—if I were saying this out loud, all statements would be in the form of questions…
Unfortunately these need to be inserted before we lower invoke statements because a component needs to have go and done ports to be invokable. reset and clk insertion can be possibly delayed but it might not be easy because the visitor pattern does not really provide a way to update component interfaces
Right; that makes sense. I wonder if those two sets of ports (go and done, which we actually have to do stuff with, and reset and clk, which we mostly just "thread through" without using in any Calyx-specific way) need two different solutions here. Still pretty stumped.
Not sure how to make progress on this issue. Maybe the best thing to do is just say that @clk and @reset are magical and implicit in the design before the clk- and reset-insertion pass run?