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

Calyx does not correctly support zero bit values #1866

Open
andrewb1999 opened this issue Jan 23, 2024 · 6 comments
Open

Calyx does not correctly support zero bit values #1866

andrewb1999 opened this issue Jan 23, 2024 · 6 comments
Labels
AMC Needed for Andrew's memory compiler S: Discussion needed Issues blocked on discussion

Comments

@andrewb1999
Copy link
Collaborator

Zero bit values are useful for cases where memories have a single value, and therefore do not need an address port. Currently memories will need idx_bits to be set to at least 1 to ensure no zero bit values are created. This makes memory generators more complicated and introduces extra unnecessary wires.

Zero bit values can be treated as if they do not exist, and should likely be removed before calling a backend (Verilog specifically does not support zero bit values). Currently, Calyx fails during verilog generation and gives an error about overflow on subtraction.

The most immediate use case for this is to allow the AMC frontend to support memories with a single value.

@andrewb1999 andrewb1999 added the AMC Needed for Andrew's memory compiler label Jan 23, 2024
@rachitnigam rachitnigam added the S: Discussion needed Issues blocked on discussion label Jan 24, 2024
@rachitnigam
Copy link
Contributor

I think at the Calyx level, this would be easy to do. The big question is how do we correctly parameterize primitives like memories to actually accept zero-bit ports. For example, the parameterization currently looks like this: https://github.com/calyxir/calyx/blob/main/primitives/memories.sv#L16

I assume if we just dump out 0 for the idx port, we'd get incorrect/failing Verilog compilation.

A possible alternative is using std_reg when you notice that the dimension of a memory is exactly one element. Would that make code generation too complex?

@andrewb1999
Copy link
Collaborator Author

I don't think that would be too complex, but can registers be external in the same way as memories?

@rachitnigam
Copy link
Contributor

Ah, if that is the only limitation then we should change registers to also accept @external attributes on them. Should be straightforward since we just need to emit readmemh and writememh calls in the simulation backend!

If that would solve the problem @andrewb1999, can you open a new issue about it? We can keep this one open as a separate question.

CC @EclecticGriffin @sampsyo since this affects semantics

@rachitnigam
Copy link
Contributor

@calyxir/semantics (because I just made a team)

@sampsyo
Copy link
Contributor

sampsyo commented Jan 27, 2024

Interesting point about the Verilog implementation… I guess the overall question is: do we need to eliminate all 0-bit signals before emitting Verilog? (That is, does Verilog (or do common Verilog implementations) not support the creation of 0-bit wires, even if they are never read or written?) That seems likely and, if so, it seems like there are two parts to coping with this:

  • Primitives themselves. The salient case, of course, is seq_mem_d1 as mentioned above; we would need to do something (perhaps in a compiler pass) to replace the seq_mem_d1 with something else (such as std_reg). I'd be tempted to say we could handle this internal to the primitive, without an actual compiler pass, but we'd need to eliminate the zero-width address port from the cell's interface too, which doesn't seem possible without a Calyx program transformation.
  • Elsewhere. The Calyx code that uses such a primitive will contain assignments to zero-width ports. The aforementioned pass would eliminate the relevant ports, so we would need to similarly remove any assignments involving those ports in client code.

Anyway, I guess all I'm saying is that this points in the direction of a compiler pass that eliminates zero-width wires, if we believe we must expunge them before emitting Verilog?

@ekiwi
Copy link
Contributor

ekiwi commented Mar 4, 2024

firrtl also has a pass to remove 0 width elements before emitting Verilog: https://github.com/ucb-bar/firrtl2/blob/b7e7fe88f10159d7350623554f85e0b05185876a/src/main/scala/firrtl2/passes/ZeroWidth.scala

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AMC Needed for Andrew's memory compiler S: Discussion needed Issues blocked on discussion
Projects
None yet
Development

No branches or pull requests

4 participants