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

Integer and boolean fixes #2

Merged
merged 3 commits into from
Jan 15, 2025
Merged

Integer and boolean fixes #2

merged 3 commits into from
Jan 15, 2025

Conversation

alastairreid
Copy link
Contributor

  • Interpreter now implements remaining integer operations
  • Changing the representation of boolean to use i1 instead of a new type asl.bool

With this, the head of the areid-mlir branch passes 26 of 119 backend tests. (Set ASL_XDSL_DIR to the xdsl-asl tree and 'make test-backend-mlir')

I don't think that there is any benefit from using asl.bool
(and, if there is, it will be easy enough to revert this change).
"""A hack to convert !asl.bool to i1 so that we can use scf.if."""

name = "asl.bool_to_i1"
class UnaryIntOp(IRDLOperation):
Copy link
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
class UnaryIntOp(IRDLOperation):
class UnaryIntOp(IRDLOperation, abc.ABC):

I've found it quite useful to leverage Python's abc module to let both the type-checker and the runtime enforce that classes designed to be abstract aren't instantiated directly, and that abstract methods are implemented by subclasses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good - adding that in an extra commit to this PR

@alastairreid alastairreid merged commit 122aeeb into main Jan 15, 2025
8 checks passed
@alastairreid alastairreid deleted the areid-int-ops2 branch January 15, 2025 12:04
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.

2 participants