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][ExpandWhens] Support properties #5373

Merged
merged 3 commits into from
Jun 13, 2023

Conversation

youngar
Copy link
Member

@youngar youngar commented Jun 12, 2023

This change adds initialization checking of Property typed ports to the ExpandWhens pass. This ensures that output properties of modules and input properties of instances are driven by a PropAssignOp.

This change adds initialization checking of Property typed ports to the
ExpandWhens pass.  This ensures that output properties of modules and
input properties of instances are driven by a PropAssignOp.
@youngar youngar added the FIRRTL Involving the `firrtl` dialect label Jun 12, 2023
@youngar youngar requested review from mikeurbach and rwy7 June 12, 2023 23:23
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.

I'm not super familiar with ExpandWhens, but adding this support looks good to me.

Copy link
Contributor

@dtzSiFive dtzSiFive left a comment

Choose a reason for hiding this comment

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

LGTM! Glad this was as easy as this, woohoo!

In the future may be useful to teach Visitor about FConnectLike to facilitate handling of all connect-like operations in one go, then this would be even easier.

Anyway, looks good, thanks for splitting these apart!

@youngar youngar merged commit 442d1ff into llvm:main Jun 13, 2023
5 checks passed
@youngar youngar deleted the firrtl-properties-expandwhens branch June 13, 2023 00:16
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

4 participants