Skip to content

Adds SerDes#49

Open
enathang wants to merge 1 commit intomainfrom
SerDes
Open

Adds SerDes#49
enathang wants to merge 1 commit intomainfrom
SerDes

Conversation

@enathang
Copy link
Contributor

Serializing from/to an arbitrary a is a common enough glue circuit that I figured I'd just add it to cores.

Right now, I'm looking for feedback on the type signatures and implementation of deserialize, serialize, and serializeWithInitialState. I'm not particularly looking for feedback on the wording of comments (subject to change) or the unit tests, which I plan to flesh out with HedgeHog etc.

I figure the more "glue" circuits we add to cores, the more users can focus on the business logic of their circuits.

@rowanG077
Copy link
Member

rowanG077 commented Oct 13, 2025

For me a key functionality of a SerDes is that it also adapts between in phase clock domains. So for example if you have an 1-to-n Deserializer you also go from n * x hz to x hz. But perhaps my definition is not the common one.

My "ideal" deseralizer would have function type(ignoring clocks and other pesky constraints):

deserializer :: forall slowDom fastDom n. n * (DomainPeriod fastDom) ~ (DomainPeriod slowDom) => Signal fastDom a => Signal slowDom (Vec n a). Or maybe with an additional Maybe. I would perhaps even say this is so foundational that it could go into prelude.

I'm not a fan of the magic conversion of types and resizing it does currently. That can be fixed by the user explicitly imo if they want that. And with such a footgun I rather make the user type it out explicitly.

@enathang
Copy link
Contributor Author

Thanks Rowan. That's exactly the feedback I was looking for.

  1. I agree that SerDes should adapt between clock domains. My two options are then a) adapt this circuit to do that or b) rename this circuit to reflect what it actually does and not change the code. I worry (perhaps unfounded) that (a) would introduce either a performance or cognitive overhead for the circuit I currently want, so I'd be inclined to go with (b) and write a separate function that does the domain crossing later. Open to disagreement though.

  2. I understand your concern about the resizing. How would you approach solving this issue? Would you simply require users to explicitly resize it, or would you include a separate function in the module to handle this conversion (preventing the user from needing to write boiler plate)?

@rowanG077
Copy link
Member

rowanG077 commented Oct 15, 2025

So if the goal for you is not to write a serdes as I described then I would say doing 1. is overkill, unless, of course, you'd like to do it :).

In your case I would call this a up/downconverter. In clash-protocols we have that naming for a similar component for packet streams

regarding the resize. I would keep make the signature upConvert :: Signal dom (Maybe a) -> Signal dom (Maybe (Vec n a)).

Then users need to handle conversion separately, before or after this function. In the case of simple bit like things it could just be a resize . pack on the output/input.

@enathang
Copy link
Contributor Author

Yea, I've noticed this is a common pattern in Haskell - why use one function when three will do. Put another way, Haskell really likes composing unopinionated functions rather than defining "opinionated functions that are what you want 85% of the time". So it naturally occurred to me to write the helper function that you PROBABLY want (because I'm used to non-Haskell), not realizing the problem can be pretty elegantly decomposed into a couple functions, so that parts can be swapped out (the conversion function, as you mention). Pretty cool imo.

I'll have a bit of a think about how I'd like to break it down.

What are your thoughts on the verb "accumulate" instead of upConvert? Would that also work as a name or does that have a different definition already in hardware? (still learning the terminology)

@t-wallet
Copy link
Collaborator

For such generic functions it would be nice if they have a configuration option that allows you to shift either LSb first or MSb first.

@rowanG077
Copy link
Member

rowanG077 commented Oct 21, 2025

I’m not against having opinionated functions, but I prefer building them on top of non-opinionated ones. A function that performs both resize and unpack, however, feels like a footgun. It makes it too easy to accidentally convert an accumulated BitVector n into any a, resulting in a possibly malformed value.

The circuit itself is definitely useful, but given the potential for misuse and how concise the manual conversion already is, I’d rather have users spell it out explicitly. If the width conversion and unpacking were more cumbersome, I’d lean the other way. In that case the function would need very clear documentation warning that it can produce malformed values if used carelessly.

Copy link
Member

@kleinreact kleinreact left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You forgot to reference the origin of these sources. Please add the following remark:

This work has been supported in part by funding from the Agentur für Innovation in der Cybersicherheit GmbH (Cyberagentur).

@enathang
Copy link
Contributor Author

The main issue for me (why I originally made the impl. opinionated) is the work one has to do on the type level. Specifically, when you have a circuit Signal dom (BitVector (n*m)) -> Signal dom (BitVector n), and you want to send in an a, you can't just resize $ pack a because the resize output size is ambiguous. So you have to do something like :: BitVector ((BitSize a 'DivRU' n) * n), which now that I type it out isn't so bad.

So maybe I've just talked myself into agreeing with you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants