dialects (arm): add first operation #3462
Conversation
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Alex Rice <alexrice999@hotmail.co.uk>
Co-authored-by: Alex Rice <alexrice999@hotmail.co.uk>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3462 +/- ##
==========================================
+ Coverage 90.21% 90.26% +0.05%
==========================================
Files 462 463 +1
Lines 57717 57858 +141
Branches 5565 5565
==========================================
+ Hits 52067 52228 +161
+ Misses 4203 4192 -11
+ Partials 1447 1438 -9 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
| @@ -2474,7 +2474,7 @@ class RM_VbroadcastsdOp(R_RM_Operation[AVXRegisterType, GeneralRegisterType]): | |||
| name = "x86.rm.vbroadcastsd" | |||
|
|
|||
|
|
|||
| class GetAnyRegisterOperation(Generic[R1InvT], IRDLOperation, X86Op): | |||
| class GetAnyRegisterOperation(Generic[R1InvT], IRDLOperation, X86Op, ABC): | |||
There was a problem hiding this comment.
What's the reason for including this change?
There was a problem hiding this comment.
That was my doing, it was just missing this annotation, and porting the equivalent to ARM meant that it was also missed, it's a small non-functional change
xdsl/dialects/arm/ops.py
Outdated
|
|
||
| from .register import IntRegisterType | ||
|
|
||
| R1InvT = TypeVar("R1InvT", bound=IntRegisterType) |
There was a problem hiding this comment.
Are there plans to have more register types? This seems like a premature generalisation at this point
There was a problem hiding this comment.
Good point, we probably don't need it right now, let's add it later.
xdsl/dialects/arm/ops.py
Outdated
|
|
||
| @abstractmethod | ||
| def assembly_line(self) -> str | None: | ||
| raise NotImplementedError() |
There was a problem hiding this comment.
I don't think there's any need to give an implementation for an abstract method? (I could be very wrong on this, perhaps @superlopuh can correct me)
There was a problem hiding this comment.
My understanding is that this is good practice. In theory, the annotation should raise an error if not overriden, but I'm not sure if that ever breaks (and what happens if the @abstractmethod gets lost somehow).
Add custom ARM assembly syntax and generic roundtrip test to filecheck. Note: stacked PR Co-authored-by: emmau678 <eu233@Emma-laptop>
Co-authored-by: Alex Rice <alexrice999@hotmail.co.uk>
Add a first op for getting a register and a filecheck Note: stacked PR --------- Co-authored-by: emmau678 <eu233@Emma-laptop> Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com> Co-authored-by: Alex Rice <alexrice999@hotmail.co.uk>
Add a first op for getting a register and a filecheck
Note: stacked PR