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

Deprecate static attribute #1429

Closed
5 of 6 tasks
rachitnigam opened this issue Apr 30, 2023 · 12 comments · Fixed by #1911
Closed
5 of 6 tasks

Deprecate static attribute #1429

rachitnigam opened this issue Apr 30, 2023 · 12 comments · Fixed by #1911
Assignees
Labels
C: Calyx Extension or change to the Calyx IL C: static-cleanup Cleanup for Static Calyx Project S: Available Can be worked upon

Comments

@rachitnigam
Copy link
Contributor

rachitnigam commented Apr 30, 2023

With the new static primitives in place, this issue is ready to be addressed. However, it is a quite pervasive change and needs a little bit of coordination:

  • Add a new @latency attribute to mark the latency of primitive interfaces. This is needed because, as discussed in rethink about the static interface #1754, primitives can efficiently support both static and dynamic interfaces.
  • Mark the @static attribute deprecated and start emitting warnings whenever it is parsed by the frontend.
  • Remove ir::NumAttr::Static from the attribute set.
    • Remove the attr-promotion pass (not sure what is does and why it needs to exist)
    • Remove the uses of the annotation in the static-promotion pass
    • Remove the compute-static analysis

This is a major task for next semester. @calebmkim, @paili0628, and I will work on it.


Original text:
Once the transition to static groups and control is completed, we should deprecate the existing "static" attribute completely and remove/change passes that make use of it. One big blocker that requires coordination with @andrewb1999 is changing the CIRCT frontend to not use the static attribute anymore.

@rachitnigam rachitnigam added S: Blocked Issue is blocked Calyx 2.0 Things that move us towards Calyx 2.0 labels Apr 30, 2023
@rachitnigam rachitnigam added this to the Static Groups milestone Apr 30, 2023
@rachitnigam rachitnigam removed the S: Blocked Issue is blocked label May 21, 2023
@rachitnigam rachitnigam added C: Calyx Extension or change to the Calyx IL S: Available Can be worked upon and removed Calyx 2.0 Things that move us towards Calyx 2.0 labels Dec 23, 2023
@rachitnigam rachitnigam pinned this issue Dec 23, 2023
@calebmkim calebmkim added the C: static-cleanup Cleanup for Static Calyx Project label Jan 8, 2024
@calebmkim
Copy link
Contributor

Remove the attr-promotion pass (not sure what is does and why it needs to exist)

@paili0628 can correct me if I'm wrong, but I think attr-promotion takes groups with a @static(n) annotation--given to it by a frontend, and upgrades it to a static<n> group. Whereas static-promotion infers static<n> from a plain, non-annotated groups. Tbh I think it probably makes sense for attr-promotion to be merged with static-promotion, but it doesn't really matter now that we're gonna deprecate @static.

@calebmkim
Copy link
Contributor

One thing I was wondering: how should frontends generate Calyx, with the @static attribute now deprecated? Obviously, if there is some very specific cycle level timing behavior you want, you should use static.

But if there isn't, do we want to generate static<n> groups but dynamic control, so that compaction opportunities can still occur? Or just dynamic groups and control, and rely on promotion and compaction? Lmk if this question makes sense.

@rachitnigam
Copy link
Contributor Author

We talked about (but don't have an open issue) on another attribute called @loose or @reschedulable on static control programs to tell the compiler that it's okay to reschedule the things.

The other option is exposing the @promotable attribute but need to think carefully about the interactions. Either way, we need some way to express "this block has a timing information available but doesn't rely on guarantees". It would be good if it is not a completely different and overlapping mechanism like @static.

@calebmkim
Copy link
Contributor

Sounds good, I can try to think about this some more and we can discuss in the Calyx meeting next Monday.

@sampsyo
Copy link
Contributor

sampsyo commented Jan 22, 2024

Just as a little preparation for this upcoming synchronous meeting: @calebmkim's proposal along these lines makes sense to me:

do we want to generate static<n> groups but dynamic control

It makes sense to me for frontends to do this (i.e., generate static "leaves" with a plain dynamic "tree"). Basically, it seems to me like static control inference works well enough that it's not clear why a frontend would ever need to generate a static control statement—with the very important exception of cases where the semantic timing guarantees matter. (That is, when interfacing with an external system.)

I could be wrong, but I think the excuse we would need to implement an externally-supported @reschedulable or whatever would be a concrete example of a case where the frontend needs to provide this hint, and automatic inference doesn't work well.

@calebmkim
Copy link
Contributor

We've had a few synchronous discussions since this post, so I'm going to try to summarize the basic takeaways/calls to action from them.

Separating Inference and Promotion

We want to separate inference and promotion into separate passes. I have a PR ready that will do that. One important problem we tackle: suppose component B instantiates component A. Suppose also that we infer component's A's latency, and use that to infer the latency of B. If we decide not to promote A, then we can the inference information in B is no longer valid. This requires a "Fix Up".

Compaction goes in between inference and promotion.

Once we have done that, we may want to insert compaction in between the inference and promotion passes. There are two challenges to keep in mind.

  1. Suppose again that component B instantiates component A. Suppose also that we infer component's A's latency, and use that to infer the latency of B. If we decide to compact A, thereby decreasing its inferred latency, then the inference information in B is no longer valid. This requires a similar "Fix Up" to the original problem.
  2. Compaction requires two things: a) timing guarantees to ensure dependencies are obeyed in our schedule and b) the ability to reschedule groups to make it more compact. a) is a property of static control but not dynamic control, while b) is a property of dynamic control but not static control. The proposal we came out of in the meeting today was the following: compaction will take in dynamic control annotated with the @promotable attribute, and it will emit static<n> code. In other words, it does the same thing as promotion, but in a more specific context and manner. Compaction will be very opportunistic (to start, we will probably compact any seq whose latency we can decrease, even if it's only by one cycle).

Names of Attributes

The proposal above suggests using @latency(n) as the attribute to attach to components' go ports. We were thinking that a slightly better name would be @interval. The reason is the following. Suppose we define a pipelined component in Calyx: the number of cycles its control flow will take is the II, not the latency, of the component. For non-pipelined components, latency and II will obviously be the same. But I think @interval is maybe a slightly better name for the attribute?

@rachitnigam
Copy link
Contributor Author

@calebmkim can we also summarize the discussion between the @loose annotation (applied to static programs) and @promotable (applied to dynamic programs)? I think that might be relevant to the "taxonomy" of static programs that we were discussing synchronously

@calebmkim
Copy link
Contributor

calebmkim commented Jan 26, 2024

Sure; iirc the context of this discussion was figuring out how to provide the timing guarantees of static programs with the rescheduling capabilities of dynamic programs. And there are two ways to represent a Calyx program that can do this: either by a) providing an attribute to dynamic programs, or b) a@loose attribute to static programs. Note that if we choose a), then the attribute needs to be stronger than @promote: it has to be something that commits us to providing some sort of timing guarantee.

There is also a separate question about Calyx programs what frontends should emit. Obviously, if they want full control of the schedule, then they should use the static<n> keyword: the main question is for when they don't care about having full control.

In the calyx-static meeting, @sampsyo suggested that we treat compaction as a specialized case of promotion: it takes in @promotable control programs and emits static<n> programs. Since compaction is usually profitable, we can apply fairly simple heuristics to decide when to promote. This avoids having to find some sort of "in between" representation for Calyx programs that we discussed.

Anything else you wanted to have down here?

@rachitnigam
Copy link
Contributor Author

@calebmkim can we update this w.r.t. #1897?

@calebmkim
Copy link
Contributor

calebmkim commented Feb 12, 2024

Sure, I've checked the boxes I've updated. Couple notes:

  • @latency renamed to @interval
  • kept compute-static bc it is needed for InferenceAnalysis (i.e., repropogating @promotable throughout the control when necessary)

I think we can probably close the issues if this sounds good to you?

@rachitnigam
Copy link
Contributor Author

Awesome! Looks like this issue is completed then? Maybe we need to update the docs for mentions of @static and update it?

@calebmkim
Copy link
Contributor

Sure, I can update docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: Calyx Extension or change to the Calyx IL C: static-cleanup Cleanup for Static Calyx Project S: Available Can be worked upon
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants