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.

Sign up for GitHub

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

Jump to bottom

[HW] Move the CombDataFlow op interface from FIRRTL to HW #7195

Merged
merged 1 commit into from
Jun 18, 2024
Merged

[HW] Move the CombDataFlow op interface from FIRRTL to HW #7195

merged 1 commit into from
Jun 18, 2024

Conversation

prithayan
Copy link
Contributor

Move the CombDataFlow op interface from FIRRTL to HW dialect.
The op interface is better suited to reside in the HW dialect, along with other interfaces like the HWModuleLike.
This makes it more convenient for non-FIRRTL dialects to implement ops using this interface, without introducing a new dependence on the FIRRTL dialect, assuming HW dependence already exists.

uenoku
uenoku reviewed Jun 18, 2024
View reviewed changes
Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

Looks fine. I'm bit worried that this belongs to HW since the interface is only used by FIRRTL passes and FieldRef is not actually used in HW but I agree the concept itself would fit better in HW.

uenoku
uenoku approved these changes Jun 18, 2024
View reviewed changes
Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

Could you open an issue for modifying default implementation not to assume sequential? LGTM otherwise

@prithayan
Copy link
Contributor Author

Thanks @uenoku , I will create the issue.

@prithayan prithayan merged commit 6cbca83 into main Jun 18, 2024
4 checks passed
@prithayan prithayan deleted the dev/pbarua/comb-op-interface branch June 18, 2024 17:55
uenoku pushed a commit that referenced this pull request Jul 3, 2024
@prithayan @uenoku
[HW] Move the CombDataFlow op interface from FIRRTL to HW (#7195)
50d8558 
Move the `CombDataFlow` op interface from `FIRRTL` to `HW` dialect.
The op interface is better suited to reside in the `HW` dialect, along with
 other interfaces like the `HWModuleLike`.
This makes it more convenient for non-`FIRRTL` dialects to implement ops using
 this interface, without introducing a new dependence on the `FIRRTL` dialect,
 assuming  `HW` dependence already exists.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uenoku uenoku uenoku approved these changes

@darthscsi darthscsi Awaiting requested review from darthscsi darthscsi is a code owner

@dtzSiFive dtzSiFive Awaiting requested review from dtzSiFive

@seldridge seldridge Awaiting requested review from seldridge seldridge is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@prithayan @uenoku