Systolic, NTT: use more helpers for cleaner eDSL code

[anshumanmohan]

Users of the eDSL frequently need to instantiate a cell and a combinational group that acts on that cell. These two items then need to passed around in tandem. Passing the same cell with different combination groups causes different values to be driven to that cell, which obviously gives different results.

A new development in the eDSL is a slew of helper functions that generate for you, in one go, a cell and a combinational group over that cell. In the future, I think users of the eDSL should just use the helpers to get the cell and the group in one go.

It would spawn more cells -- one per group, without an opportunity to reuse the same primitive cell for multiple combinational groups -- but that should be compiled away anyway, no?

To give a concrete example, gen_msb.py does:

neq = comp.cell("neq", Stdlib.op("neq", width, signed=False))

with comp.comb_group("cur_val_cond") as cur_val_cond:
        neq.left = const(width, 0)
        neq.right = cur_val.out

with comp.comb_group("count_cond") as count_cond:
        neq.left = const(width, 0)
        neq.right = counter.out

while_with(
            CellAndGroup(neq, cur_val_cond),
            [incr_count, shift_cur_val],
        ),
...        
while_with(
            CellAndGroup(neq, count_cond),
            [decr_count, shift_val_build],
        ),

And I am proposing that it should instead do:

cur_val = comp.neq_use(const(width, 0), cur_val.out, width)
count = comp.neq_use(const(width, 0), counter.out, width)

while_with(
            cur_val,
            [incr_count, shift_cur_val],
        ),
...        
while_with(
            count,
            [decr_count, shift_val_build],
        ),

Where, in my proposal, cur_val and count are now tuples (technically objects of the dataclass CellAndGroup), which we don't unwrap and rewrap. They have their own neq cells, which is luxurious and in this case wasteful. But let's realize that fact later and compile it away?

_Originally posted by @anshumanmohan in https://github.com/cucapra/calyx/pull/1666#discussion_r1297443278_

[anshumanmohan]

Ah actually there is a middle ground that saves on spurious primitive cells!

They can use the slightly lower-level helper binary_use, which takes a cell and two inputs, and drives those two inputs to that cell. The user can now provide the same neq both times. The result will still come nicely wrapped up, and they'll be able to pass it around wholesale without having to unwrap it.

neq = comp.neq(width, "neq")

cur_val = comp.binary_use(const(width, 0), cur_val.out, neq)
count = comp.binary_use(const(width, 0), counter.out, neq)

...

[rachitnigam]

Yeah, this looks like a cool way to use the cells. This is besides the point you were making but it's rarely worth sharing combinational components like neq precisely because they are so cheap but obviously you'd want to share things like multipliers and more complex hardware.

[sampsyo]

Yes! Just to clarify, I think the libraries already support this, so this issue is just about changing the clients to use the more convenient *_use stuff, right?

[anshumanmohan]

Yes, either binary_use or the more specific <op>_use can be tossed in there right now!

Putting binary_use in there would produce the same Calyx code as the clients do currently. The use of the more specific <op>_use would produce cleaner eDSL code, but would spawn extra op components like eq, neq, etc in the Calyx code.

I'll go through and do the first option in case of expensive operations and the second in case of simpler operations!

[sampsyo]

I wouldn't worry too much about duplicating stuff! I think most (all?) cases will use std_add at most, for which duplication is unlikely to be a problem. And if nothing else, it is just more work for the sharing pass to do…

[rachitnigam]

I think it would be useful to open an issue to change the systolic and NTT frontends to use this new interface just so that we can have more users of the library and understand potential pain points.

[anshumanmohan]

Yup, this is now that issue! I’ll just change the name…

[anshumanmohan]

In #2034 and #2036, I have made changes of the sort proposed above in the NTT and Systolic frontends respectively. I'd be happy to hear of other big chunks of eDSL code that could use similar treatment!

[anshumanmohan]

Okay I've taken a look at the other frontends and I'm pretty sure there's not a ton to be done in those cases. MrXL could be reworked a ton, but at pedagogical cost. I say we leave it as it is. If we want, we can add a line to the relevant part of the frontend tutorial saying that the eDSL code has been written in a verbose manner.

Anyway, maybe this issue can be closed when #2034 is merged.