-
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 (arm): add mov op #3476
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3476 +/- ##
==========================================
+ Coverage 90.26% 90.33% +0.06%
==========================================
Files 462 464 +2
Lines 57820 58157 +337
Branches 5565 5557 -8
==========================================
+ Hits 52192 52536 +344
+ Misses 4191 4190 -1
+ Partials 1437 1431 -6 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
xdsl/dialects/arm/ops.py
Outdated
@irdl_op_definition | ||
class DSMovOp(ARMOperation): | ||
""" | ||
Copies the value of r1 into r2. |
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.
what are r1
and r2
?
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.
thanks, this should be operand(2) and rd
xdsl/dialects/arm/ops.py
Outdated
name = "arm.ds.mov" | ||
|
||
rd = result_def(IntRegisterType) | ||
operand2 = operand_def(IntRegisterType) |
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.
Having an operand2
but no operand
feels odd
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 just following the ARM documentation for this instruction (https://developer.arm.com/documentation/dui0473/m/arm-and-thumb-instructions/mov), but I can change it
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
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.
Have put one small comment (which is more of a personal preference thing so feel free to ignore) then this looks good to merge from my end.
xdsl/dialects/arm/ops.py
Outdated
s: Operation | SSAValue, | ||
*, | ||
comment: str | StringAttr | None = None, | ||
d: IntRegisterType, |
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.
I would personally put this as a positional argument.
Add assembly printing for ARM instructions Note: stacked PR --------- Co-authored-by: emmau678 <eu233@Emma-laptop> Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Add a mov op that moves data from source to destination --------- Co-authored-by: emmau678 <eu233@Emma-laptop> Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Add a mov op that moves data from source to destination