Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make Queue.irrevocable work properly in chisel3 #1136

Merged
merged 1 commit into from
Feb 10, 2020
Merged

Make Queue.irrevocable work properly in chisel3 #1136

merged 1 commit into from
Feb 10, 2020

Conversation

edwardcwang
Copy link
Contributor

Close #1134

Type of change: bugfix

Impact: no functional change

Development Phase: implementation

Release Notes: Fix unexpected error from attempting to use Queue.irrevocable in chisel3.

@edwardcwang edwardcwang added DO NOT MERGE Bugfix Fixes a bug, will be included in release notes labels Jul 28, 2019
@edwardcwang edwardcwang requested a review from a team as a code owner July 28, 2019 03:23
@edwardcwang edwardcwang force-pushed the queue3 branch 3 times, most recently from 7d33860 to 375e229 Compare July 28, 2019 08:07
@edwardcwang edwardcwang requested a review from jackkoenig July 28, 2019 08:38
@edwardcwang
Copy link
Contributor Author

This is ready for review.

@edwardcwang edwardcwang mentioned this pull request Jul 28, 2019
@seldridge seldridge self-assigned this Aug 11, 2019
@seldridge seldridge self-requested a review August 11, 2019 04:03
Copy link
Member

@seldridge seldridge left a comment

Choose a reason for hiding this comment

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

Looks good. We really need full coverage of all methods/classes and this helps!

I think the test can be pared down to strictly test elaboration. There's likely no need to do a full-blown Verilator run here, considering an irrevocable queue is just a shim around a queue returning a different interface. I provided one example of how I would do this as a suggestion.

@claassistantio
Copy link

claassistantio commented Feb 10, 2020

CLA assistant check
All committers have signed the CLA.

@edwardcwang
Copy link
Contributor Author

@seldridge Fixed after a long delay ;)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Bugfix Fixes a bug, will be included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Queue.irrevocable
3 participants