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: (arith) adding an arith canonicalisation pattern #2094

Merged
merged 19 commits into from
Feb 6, 2024

Conversation

dshaaban01
Copy link
Collaborator

PR 2 out of 4.

Added an arith canonicalisation pattern to use as an example/ starting point to extending arith canonicalisation patterns.

@dshaaban01 dshaaban01 added tool interactive xdsl-gui things labels Feb 5, 2024
@dshaaban01 dshaaban01 self-assigned this Feb 5, 2024
@dshaaban01 dshaaban01 changed the base branch from main to dalia/interactive_individual_rewrite_1 February 5, 2024 23:28
@superlopuh superlopuh changed the title interactive: adding an arith canonicalisation pattern dialects: (arith) adding an arith canonicalisation pattern Feb 6, 2024
@superlopuh superlopuh added dialects Changes on the dialects and removed tool interactive xdsl-gui things labels Feb 6, 2024
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.

can you please add a filecheck test for this?

Copy link

codecov bot commented Feb 6, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (b9b208c) 90.13% compared to head (21337b0) 90.12%.

Files Patch % Lines
xdsl/transforms/canonicalization_patterns/arith.py 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                            Coverage Diff                             @@
##           dalia/interactive_individual_rewrite_1    #2094      +/-   ##
==========================================================================
- Coverage                                   90.13%   90.12%   -0.01%     
==========================================================================
  Files                                         296      297       +1     
  Lines                                       36799    36779      -20     
  Branches                                     5464     5461       -3     
==========================================================================
- Hits                                        33168    33148      -20     
+ Misses                                       2853     2852       -1     
- Partials                                      778      779       +1     

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

@dshaaban01 dshaaban01 requested a review from superlopuh February 6, 2024 17:51
Base automatically changed from dalia/interactive_individual_rewrite_1 to main February 6, 2024 22:09
@dshaaban01 dshaaban01 merged commit cb08c91 into main Feb 6, 2024
10 checks passed
@dshaaban01 dshaaban01 deleted the dalia/interactive_add_arith_canon_2 branch February 6, 2024 22:10
Comment on lines +66 to +68
pattern.match_and_rewrite(matched_operation, rewriter)
if not rewriter.has_done_action:
raise ValueError("Invalid rewrite at current location.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

To me, this feels like a case that should have a warning instead of an exception. It feels like this might make it hard to interact with this pass through scripts and stuff.
Though, I guess people consider exceptional control flow for basic stuff acceptable in Python, so this is just a side comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialects Changes on the dialects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants