-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add Arm64 encoding for IF_SVE_BL_1A #97223
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsThese instructions use a pattern to specify which elements to count from a predicate. This is encoded as 5 bits in the instruction. I've added a new entry in New clr test output (identical to capstone):
|
@kunalspathak @dotnet/arm64-contrib |
17504dc
to
1563f8c
Compare
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 looks good imo. I see why you have to introduce the new entry because you have to flow that info to the part of the actual encoding.
I'd wait for Kunal's review too before merging.
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.
A minor comment. Otherwise, LGTM
}; | ||
|
||
insSvePattern _idSvePattern; |
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 looks ok to me, but wondering why not just have something like this after the end of struct
definition.
#ifdef TARGET_ARM64
insSvePattern _idSvePattern;
#endif
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.
Happy to switch to that.
My reasoning was when you look at the whole file we have an #ifdef
for each target. Many of them just have the same thing (_idReg3
and _idReg4
). The nesting of the Arm32/Arm64 is a little messy to read and doesn't fit the rest of the file style. Splitting it made it simpler.
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.
LGTM
These instructions use a pattern to specify which elements to count from a predicate. This is encoded as 5 bits in the instruction. I've added a new entry in
instrdesc
union (to prevent it increasing the size).Imm encoding is a 4bit number representing 1 to 16, so 1 needs subtracting when encoding.
New clr test output (identical to capstone):