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

core: Fix terminator conditions for unregistered operations #1201

Merged
merged 5 commits into from Jun 28, 2023

Conversation

compor
Copy link
Collaborator

@compor compor commented Jun 27, 2023

This PR refines terminator conditions for unregistered operations.
Namely, if we allow unregistered operations, then their terminator conditions are implicitly fulfilled.
It also modifies has_trait to assume the existence of any trait for unregistered operations.

This is modelled after the mayBeValidWithoutTerminator and mightHaveTrait in MLIR.

Resolves #1167

@compor compor added the core xDSL core (ir, textual format, ...) label Jun 27, 2023
@compor compor requested a review from superlopuh June 27, 2023 12:20
@compor compor requested a review from math-fehr as a code owner June 27, 2023 12:20
@compor compor self-assigned this Jun 27, 2023
@codecov
Copy link

codecov bot commented Jun 27, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (00608a1) 88.71% compared to head (0a60692) 88.71%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1201   +/-   ##
=======================================
  Coverage   88.71%   88.71%           
=======================================
  Files         166      166           
  Lines       22855    22858    +3     
  Branches     3478     3479    +1     
=======================================
+ Hits        20275    20278    +3     
  Misses       2025     2025           
  Partials      555      555           
Impacted Files Coverage Δ
xdsl/ir/core.py 87.76% <100.00%> (+0.04%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

xdsl/ir/core.py Show resolved Hide resolved
Copy link
Collaborator

@math-fehr math-fehr left a comment

Choose a reason for hiding this comment

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

Nice!
I just pushed #1202, which should move the allow_unregistered to the MLContext. This means that if allow_unregistered is not set, then we cannot create UnregisteredOp in the first place, so we don't have to pass this argument to verify anymore.

xdsl/ir/core.py Outdated Show resolved Hide resolved
xdsl/ir/core.py Show resolved Hide resolved
xdsl/ir/core.py Outdated Show resolved Hide resolved
@compor
Copy link
Collaborator Author

compor commented Jun 27, 2023

Nice! I just pushed #1202, which should move the allow_unregistered to the MLContext. This means that if allow_unregistered is not set, then we cannot create UnregisteredOp in the first place, so we don't have to pass this argument to verify anymore.

I'll update once #1202 is in! Thanks

@math-fehr
Copy link
Collaborator

Nice! I just pushed #1202, which should move the allow_unregistered to the MLContext. This means that if allow_unregistered is not set, then we cannot create UnregisteredOp in the first place, so we don't have to pass this argument to verify anymore.

I just merged #1202 by the way!

@compor compor force-pushed the christos/core/term-traits-for-unregistered branch from d4e7e01 to ac2a8ee Compare June 28, 2023 10:06
@compor compor requested a review from math-fehr June 28, 2023 10:13
Copy link
Collaborator

@math-fehr math-fehr left a comment

Choose a reason for hiding this comment

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

Perfect, thanks a lot!

@math-fehr math-fehr merged commit e54676e into main Jun 28, 2023
12 checks passed
@math-fehr math-fehr deleted the christos/core/term-traits-for-unregistered branch June 28, 2023 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core xDSL core (ir, textual format, ...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix terminator conditions for unregistered operations
3 participants