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

transformations: (cf) switch canonicalization #3291

Merged
merged 6 commits into from
Oct 15, 2024
Merged

Conversation

alexarice
Copy link
Collaborator

Adds all the switch canonicalization patterns present in mlir. I'm happy to anonymise some of the block names in the filecheck tests if this is deemed worth doing.

@alexarice alexarice added dialects Changes on the dialects transformations Changes or adds a transformatio labels Oct 11, 2024
@alexarice alexarice self-assigned this Oct 11, 2024
Copy link

codecov bot commented Oct 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.02%. Comparing base (b452684) to head (51436cf).
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3291   +/-   ##
=======================================
  Coverage   90.01%   90.02%           
=======================================
  Files         445      445           
  Lines       55956    56049   +93     
  Branches     5364     5378   +14     
=======================================
+ Hits        50370    50458   +88     
- Misses       4177     4180    +3     
- Partials     1409     1411    +2     

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

Copy link
Member

Choose a reason for hiding this comment

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

nit: it's slightly confusing to have the block indices be different in the inputs and outputs

-> br ^bb2
"""
case_values = switch.case_values
if case_values is not None:
Copy link
Member

Choose a reason for hiding this comment

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

does it make sense for this to be optional? Shouldn't it be () by default?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is an optional property in mlir. I don't personally think it makes any sense but it is what it is

@alexarice alexarice force-pushed the alexarice/switch-canon branch from 07fd41f to fc0e077 Compare October 15, 2024 13:02
@alexarice alexarice force-pushed the alexarice/switch-canon branch from 2ea46d1 to 51436cf Compare October 15, 2024 13:23
@alexarice
Copy link
Collaborator Author

@superlopuh happy for this to be merged?

Copy link
Member

@superlopuh superlopuh left a comment

Choose a reason for hiding this comment

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

Thank you!

@alexarice alexarice merged commit 7e5abb8 into main Oct 15, 2024
14 checks passed
@alexarice alexarice deleted the alexarice/switch-canon branch October 15, 2024 21:22
EdmundGoodman pushed a commit to EdmundGoodman/xdsl that referenced this pull request Dec 6, 2024
Adds all the switch canonicalization patterns present in mlir. 

---------

Co-authored-by: Sasha Lopoukhine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialects Changes on the dialects transformations Changes or adds a transformatio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants