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

[DC] Add cast to/from ESI ops #5676

Merged
merged 2 commits into from
Jul 28, 2023
Merged

[DC] Add cast to/from ESI ops #5676

merged 2 commits into from
Jul 28, 2023

Conversation

mortbopet
Copy link
Contributor

Much needed cast operations to/from ESI.
Given that dc.values have unnamed fields (which i think is a good descision) dc.values with multiple types will be converted to a hw.struct with an inferred name, in this case field#. In most cases, this won't be compatible with the structs used in an input/output ESI channel. To address this, source materializations (esi -> dc) are automatically handled by an inserted hw.bitcast. Target materializations (and result type inference, dc -> esi) can be directed by specifying a forced type in the dc.to_esi op.

@mortbopet mortbopet changed the title [DC] Add to/from ESI ops [DC] Add cast to/from ESI ops Jul 25, 2023
@teqdruid
Copy link
Contributor

What is the rationale for dc.value allowing multiple value types? Isn't that just implicitly a tuple? If so, we should just have a tuple type. I get that this would be one more op for scenarios wherein multiple values are being packed, but it is (IMO) a better separation of concerns.

Much needed cast operations to/from ESI.
Given that `dc.value`s have unnamed fields (which i think is a good descision) `dc.value`s with multiple types will be converted to a `hw.struct` with an inferred name, in this case `field#`.
In most cases, this won't be compatible with the structs used in an input/output ESI channel. To address this, source materializations (esi -> dc) are automatically handled by an inserted `hw.bitcast`. Target materializations (and result type inference, dc -> esi) can be directed by specifying a forced type in the `dc.to_esi` op.
@mortbopet
Copy link
Contributor Author

With the simplification of dc.value, this commit has been reduced to something trivial, so i'll go ahead and merge.

@mortbopet mortbopet merged commit 2bbd538 into main Jul 28, 2023
5 checks passed
Copy link
Contributor

@teqdruid teqdruid left a comment

Choose a reason for hiding this comment

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

This is far simpler than it was. I post-merge commit approve.

@darthscsi darthscsi deleted the dev/mpetersen/dc_esi branch June 4, 2024 14:49
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.

3 participants