-
Notifications
You must be signed in to change notification settings - Fork 46
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
Omit the @clk and @reset ports when inlining cell ports to the component signature #1034
Comments
The problem that @paili0628 ran into happens because 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 |
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 |
Right; that makes sense. I wonder if those two sets of ports ( |
Not sure how to make progress on this issue. Maybe the best thing to do is just say that |
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 theclk
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 text was updated successfully, but these errors were encountered: