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

[FIRRTL] Add property assign op instead of using connect #5361

Merged
merged 3 commits into from
Jun 12, 2023

Conversation

youngar
Copy link
Member

@youngar youngar commented Jun 12, 2023

This adds a new statement to FIRRTL to assign a property to a property sink. We stop allowing properties to be connected with a ConnectOp. This new operation enforces that the parent operation is a FModuleOp, and that it has static single connects, which is a good limitation to keep from using this operation inside a WhenOp.

This adds a new statement to FIRRTL to assign a property to a proptery
sink.  We stop allowing properties to be connected with a ConnectOp.
This new operation enforces that the parent operation is a FModuleOp,
and that it has static single connects, which is a good limitation to
keep from using this operation inside a WhenOp.
@youngar youngar added the FIRRTL Involving the `firrtl` dialect label Jun 12, 2023
@youngar youngar requested review from mikeurbach and rwy7 June 12, 2023 18:26
Co-authored-by: Robert Young <rwy0717@gmail.com>
Copy link
Contributor

@mikeurbach mikeurbach left a comment

Choose a reason for hiding this comment

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

Just some op name bikeshedding, but otherwise looks good to me!

include/circt/Dialect/FIRRTL/FIRRTLStatements.td Outdated Show resolved Hide resolved
@youngar youngar merged commit 5866e78 into llvm:main Jun 12, 2023
5 checks passed
@youngar youngar deleted the firrtl-passign branch June 12, 2023 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FIRRTL Involving the `firrtl` dialect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants