Skip to content

arith: replace string predicates with CmpIPredicate enums in ArithVal… - #707

Open
viniciusfdasilva wants to merge 4 commits into
llvm:mainfrom
viniciusfdasilva:main
Open

viniciusfdasilva wants to merge 4 commits into
llvm:mainfrom
viniciusfdasilva:main

Conversation

@viniciusfdasilva

Copy link
Copy Markdown

This PR refactors ArithValue's comparison operators to dispatch on CmpIPredicate enum members instead of ad-hoc strings, removing the string-based predicate mini-language while preserving existing signed/unsigned and int/float resolution behavior.

Resolves #230

@makslevental
makslevental self-requested a review September 25, 2026 02:42


canonicalizer = ArithCanonicalizer()
canonicalizer = ArithCanonicalizer() No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you restore the newline here

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Of course! Sorry for this!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This code snippet was already in arith.py! I think there was an extra blank line, so I removed it!

Comment thread projects/eudsl-python-extras/mlir/extras/dialects/arith.py Outdated

@makslevental makslevental left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks like you have some issues

viniciusfdasilva and others added 2 commits September 25, 2026 09:26
Co-authored-by: Maksim Levental <maksim.levental@gmail.com>
Removed an unnecessary blank line in the ArithCanonicalizer class.
Ensure proper instantiation of ArithCanonicalizer.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TODO] migrate ArithValue to use cmp enums themselves instead of strings.

2 participants