-
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
Arm64/SVE: Add Uzp1 #94529
Arm64/SVE: Add Uzp1 #94529
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsThis is similar to #93067, except that most of the code added in this PR was generated by the tool, the same tool that generated encodings in #94285. The only thing that I realized the tool does not generate is the entries in I have also added method placeholders for predicate registes which will need to be fixed once we add predicate register support in LSRA. I think the next step for this PR would be to make sure the encodings are correctly generated by comparing it with cordistool or lldb. That won't be possible until we add the actual Z and predicate registers. Contributes to #93095
|
@@ -10571,6 +10619,47 @@ void emitter::emitIns_Call(EmitCallType callType, | |||
return ureg << 10; | |||
} | |||
|
|||
/***************************************************************************** | |||
* | |||
* Returns an encoding for the specified register used in the 'Vd' position |
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.
* Returns an encoding for the specified register used in the 'Vd' position | |
* Returns an encoding for the specified register used in the 'Pd' position |
|
||
/***************************************************************************** | ||
* | ||
* Returns an encoding for the specified register used in the 'Vn' position |
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.
* Returns an encoding for the specified register used in the 'Vn' position | |
* Returns an encoding for the specified register used in the 'Pn' position |
|
||
/***************************************************************************** | ||
* | ||
* Returns an encoding for the specified register used in the 'Vm' position |
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.
* Returns an encoding for the specified register used in the 'Vm' position | |
* Returns an encoding for the specified register used in the 'Pm' position |
I have a WIP patch which adds one SVE API entry. I'll rebase it on top on this and switch it to use UZP1. |
src/coreclr/jit/codegenarm64.cpp
Outdated
@@ -6702,6 +6702,14 @@ void CodeGen::genArm64EmitterUnitTests() | |||
theEmitter->emitIns_R_R_R(INS_asrv, EA_4BYTE, REG_R8, REG_R9, REG_R10); | |||
theEmitter->emitIns_R_R_R(INS_rorv, EA_4BYTE, REG_R8, REG_R9, REG_R10); | |||
|
|||
// TODO-SVE: Fix once we add predicate registers | |||
theEmitter->emitIns_R_R_R(INS_uzp1, EA_8BYTE, REG_R0, REG_R1, REG_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.
This is emitting a neon uzp1
, it should be INS_sve_uzp1
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.
good catch. i will address this and fix the tool.
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.
fixed and update the file in #94549
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.
With the above change, you're going to have to add case INS_sve_uzp1
into emitIns_R_R_R()
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.
yes and many such cases in emitIns*
methods.
This is similar to #93067, except that most of the code added in this PR was generated by the tool, the same tool that generated encodings in #94285. The only thing that I realized the tool does not generate is the entries in
emitInsMayWriteToGCReg()
. I could add it, but I think it is easier to do it manually.I have also added method placeholders for predicate registes which will need to be fixed once we add predicate register support in LSRA. I think the next step for this PR would be to make sure the encodings are correctly generated by comparing it with cordistool or lldb. That won't be possible until we add the actual Z and predicate registers.
Another TODO would be to fix the PerfScore numbers, not sure where to check that information though.
Contributes to #93095