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

dialects: (seq) Add builder and verifier for CompRegOp #2429

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

Moxinilian
Copy link
Contributor

This PR adds a verifier and a builder to CompRegOp.

@Moxinilian Moxinilian added enhancement New feature or request dialects Changes on the dialects labels Apr 4, 2024
@Moxinilian Moxinilian self-assigned this Apr 4, 2024
Copy link
Collaborator

@AntonLydike AntonLydike left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution, I just added a few comments on minor things I'd really like to see improved before merging.

Also, how do magnets work? 🤔

tests/dialects/test_seq.py Show resolved Hide resolved
xdsl/dialects/seq.py Show resolved Hide resolved
if (self.reset is not None and self.reset_value is None) or (
self.reset_value is not None and self.reset is None
):
raise VerifyException("both reset and reset_value must be set when one is")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Our verifier messages usually start with uppercase letters afaik.

Copy link

codecov bot commented Apr 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.71%. Comparing base (ba19281) to head (23e549d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2429   +/-   ##
=======================================
  Coverage   89.70%   89.71%           
=======================================
  Files         345      345           
  Lines       42001    42014   +13     
  Branches     6251     6252    +1     
=======================================
+ Hits        37678    37691   +13     
  Misses       3407     3407           
  Partials      916      916           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@AntonLydike AntonLydike left a comment

Choose a reason for hiding this comment

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

LGTM 📈

@Moxinilian Moxinilian merged commit e5f8921 into xdslproject:main Apr 4, 2024
9 checks passed
@Moxinilian Moxinilian deleted the compreg branch April 4, 2024 15:41
@Moxinilian Moxinilian restored the compreg branch April 4, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialects Changes on the dialects enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants