calyxir / calyx

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

eDSL: Make register names optional #2010

Closed anshumanmohan closed 2 weeks ago

anshumanmohan commented 3 weeks ago

For the most part, we can allow the builder library to just pick a name for our registers. There is already a simple name-generator that we use to name new cells and such, so this isn't hard to do.

The only point when we really must be careful is when a register is to be passed by reference. In that case, the invoking component really does need to know the string name of the register. To make this happen, we do name the register in the invoked component using the optional name field. We then use the same name in the invoking component's invoke command.

anshumanmohan commented 3 weeks ago

Giving up because it would take far too much work to change this all over the place. Lots of "old-fashioned" uses of the DSL use string-name-based accesses of registers, meaning that we do need to pass string names and keep them straight throughout the program.

IMO my overall observation is reasonable: registers should not be given names unless necessary, and it's only really necessary when they are ref cells. Maybe this is just a good thing to keep in mind when writing modern eDSL code.

rachitnigam commented 3 weeks ago

Thanks for taking a shot at this @anshumanmohan! Any thoughts on a possible migration path? We can define a new method and rename the old one into an internal API and eventually deprecate it? I think in general, we should evolve the eDSL aggressively (especially to make the scheduling stuff easier) and have a deprecation path of the existing code.

anshumanmohan commented 3 weeks ago

It’s not actually too hard to just muscle through this change; I just gave up because I viewed it as a nicety that was perhaps not worth the splashy PR. In this specific case I may just change the order everywhere, add a Python-level requirement that ref registers must have a name, and add a stylistic suggestion in the docs that cells not be named unless necessary.

Let’s step back from this specific issue. The old-fashioned eDSL code relies heavily on janky “bare” accesses, which allow you to mention all kinds of components, groups, and cells by string name. That’s the real source of ugliness and fragility IMO. I thiiink that all that old code can already be rewritten in the modern style, where Python handles are used instead, but I’d need to check carefully. It is possible that there’s something going on with that code that the eDSL just can’t currently support without bare accesses. In that case we should explore an expansion to the eDSL.

anshumanmohan commented 3 weeks ago

@rachitnigam I did it haha

For the most part I have just brainlessly swapped the order of the size and the name. Many of those names are not necessary and can be generated. Many more are necessary at present, because of old-fashion eDSL writing, but can probably be massaged away.