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

[Handshake] Improve assembly format of BundleOp and UnbundleOp #99

Closed
lucas-rami opened this issue Jul 9, 2024 · 0 comments
Closed
Labels
enhancement Improvement to existing feature of Dynamatic nice-to-have Cosmetic and/or light improvements to the framework

Comments

@lucas-rami
Copy link
Member

The initial assembly format introduced as part of 9fef8a5 for the two bundling-related operations is very verbose, especially when used for channel types with extra signals. In particular, there is redundancy between the channel type and the type of the individual signals that are being bundled into or unbundled from it. There is also redundancy between the operand types and the result types; the exact handshake::ChannelType or handshake::ControlType involved in the operation is sufficient to characterize the type of all operands/results uniquely.

Below are proposed simplification examples.

// Old
%channel, %extra = bundle %ctrl, %data : (!handshake.control, i32) -> (!handshake.channel<i32, [extra: i1]>, i1)
// Proposed
%channel, %extra = bundle %ctrl, %data : ... -> !handshake.channel<i32, [extra: i1]>

// -----

// Old
%ctrl, %data = unbundle %channel [%extra] : (!handshake.channel<i32, [extra: i2 (U)]>) -> (!handshake.control, i32, i2)
// Proposed
%ctrl, %data = unbundle %channel [%extra] : !handshake.channel<i32, [extra: i2 (U)]> -> ...

Implementing the above syntax requires custom parsers/printers, as ODS won't let us specify this declaratively.

This is related to the type system redesign (#86).

@lucas-rami lucas-rami added enhancement Improvement to existing feature of Dynamatic nice-to-have Cosmetic and/or light improvements to the framework labels Jul 9, 2024
Liudmila-Paskonova pushed a commit that referenced this issue Aug 7, 2024
The assembly format of the `handshake::BundleOp` and
`handshake::UnbundleOp` operations was very verbose; in particular iot
contained a lot of redundancy in the types of all operands/results, when
a single control/channel type is in fact enough to encode the type of
all operands/results.

This commit implements custom (similar) printers/parsers for both
operations, which consequentially simplify the ops' printed format and
additionally reduces the number of potential issues with the ops' text
serialization (hence the removal of some unit tests, in addition to the
modification of others).

Fixes #99.
Liudmila-Paskonova pushed a commit that referenced this issue Aug 23, 2024
The assembly format of the `handshake::BundleOp` and
`handshake::UnbundleOp` operations was very verbose; in particular iot
contained a lot of redundancy in the types of all operands/results, when
a single control/channel type is in fact enough to encode the type of
all operands/results.

This commit implements custom (similar) printers/parsers for both
operations, which consequentially simplify the ops' printed format and
additionally reduces the number of potential issues with the ops' text
serialization (hence the removal of some unit tests, in addition to the
modification of others).

Fixes #99.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to existing feature of Dynamatic nice-to-have Cosmetic and/or light improvements to the framework
Projects
None yet
Development

No branches or pull requests

1 participant