-
Notifications
You must be signed in to change notification settings - Fork 77
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) add truncation option to ConstantOp #3635
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3635 +/- ##
=======================================
Coverage 90.74% 90.74%
=======================================
Files 465 465
Lines 58750 58755 +5
Branches 5630 5630
=======================================
+ Hits 53312 53317 +5
Misses 3997 3997
Partials 1441 1441 ☔ View full report in Codecov by Sentry. |
uv.lock
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uh oh, let's separate this out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is my bad, I think the new workflow should be to make this change a part of the dependabot PR, I'll do this quickly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah no I merged textual, you have a newer version of ruff than our requirements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not ideal that switching between branches messes up lock files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory, this is exactly what UV is supposed to enable, switching branches with different dependencies and updating those silently
@staticmethod | ||
def from_int_and_width( | ||
value: int | IntAttr, value_type: int | IntegerType | IndexType | ||
value: int | IntAttr, | ||
value_type: int | IntegerType | IndexType, | ||
*, | ||
truncate_bits: bool = False, | ||
) -> ConstantOp: | ||
if isinstance(value_type, int): | ||
value_type = IntegerType(value_type) | ||
return ConstantOp.create( | ||
result_types=[value_type], | ||
properties={"value": IntegerAttr(value, value_type)}, | ||
properties={ | ||
"value": IntegerAttr(value, value_type, truncate_bits=truncate_bits) | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please add a unit test for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TY
split off from #3583