calyxir / calyx

Intermediate Language (IL) for Hardware Accelerator Generators
https://calyxir.org
MIT License
453 stars 45 forks source link

Support `new_fsm` attribute for tdst pass #1363

Closed calebmkim closed 2 weeks ago

calebmkim commented 1 year ago

In the same way that the new_fsm attribute prevents fsm complexity blow up in the tdcc pass, we might want to add support for the attribute when compiling tdst as well. I can try to implement this, since it will probably be helpful for me to better understand tdst, especially if I'm trying to implement some of the new static group syntax (w/ the % relative clock cycles thing).

rachitnigam commented 1 year ago

You can take a shot but the big problem is that the static FSMs don't reset correctly which means you can't compositionally compile the program. This is why our current implementation is so restrictive (one component, completely static).

The problem is that there is no good way (that I know of) to build a static FSM that provides both a done condition and resets correctly so that it can be re-executed in the same cycle.

To observe this problem, you can run the code mentioned in the fix-tdst PR. Uncomment the code and delete the stuff before and after the loop. If you look at the generated VCD, you'll notice that the inner if branches don't reset in time to be re-executed every cycle.

calebmkim commented 1 year ago

Ah I see. Maybe this is something that would be better to implement once the static group stuff is more developed.

rachitnigam commented 1 year ago

I'm going to make this task blocked on the static timing implementation

rachitnigam commented 1 year ago

Blocked on #1344

rachitnigam commented 11 months ago

@calebmkim did we decide that this doesn't need to be solved or that it isn't obvious what it means in the context of the new static lowering pass?

calebmkim commented 11 months ago

We never fully reached a conclusion, but rn I'm leaning towards it not needing to be solved. I explain why I think that here. (although in the comment, you mention that it's possible that there may be some benefit to inlining static control w/ new fsms).

rachitnigam commented 11 months ago

Right! Maybe we should mark this as a low priority QoR thing to investigate if we have the time?

calebmkim commented 2 weeks ago

This discussion is subsumed by the overall FSM optimizations discussion described in #2020.