-
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 encodings for group IF_SVE_CC,CD #99284
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsAdd encodings for the Matching capstone output:
Contributing towards #94549
|
@a74nh @kunalspathak @dotnet/arm64-contrib |
INS_OPTS_SCALABLE_D); // INSR <Zdn>.<T>, <V><m> | ||
|
||
// IF_SVE_CD_2A | ||
theEmitter->emitIns_R_R(INS_sve_insr, EA_SCALABLE, REG_V4, REG_R23, |
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.
As per Insr, this also takes ZR
register. Can you please add a test for that?
src/coreclr/jit/emitarm64.cpp
Outdated
case IF_SVE_CD_2A: // ........xx...... ......mmmmmddddd -- SVE insert general register | ||
assert(insOptsScalable(id->idInsOpt())); | ||
assert(isVectorRegister(id->idReg1())); // ddddd | ||
assert(isGeneralRegister(id->idReg2())); // mmmmm |
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.
assert(isGeneralRegister(id->idReg2())); // mmmmm | |
assert(isGeneralRegisterOrZR(id->idReg2())); // mmmmm |
src/coreclr/jit/emitarm64.cpp
Outdated
@@ -3244,6 +3256,28 @@ static const char * const qRegNames[] = | |||
"q30", "q31" | |||
}; | |||
|
|||
static const char * const dRegNames[] = |
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.
these are already aliased at
runtime/src/coreclr/jit/registerarm64.h
Line 64 in fb700b3
REGDEF(V0, 0+VBASE, VMASK(0), "d0", "s0") |
src/coreclr/jit/emitarm64.cpp
Outdated
// Return value: | ||
// A string that represents a SIMD scalar register name. | ||
// | ||
const char* emitter::emitSimdScalarRegName(regNumber reg, emitAttr attr) |
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.
src/coreclr/jit/emitarm64.cpp
Outdated
{ | ||
fmt = IF_SVE_CC_2A; | ||
} | ||
else if (isGeneralRegister(reg2)) |
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.
else if (isGeneralRegister(reg2)) | |
else if (isGeneralRegisterOrZR(reg2)) |
src/coreclr/jit/emitarm64.cpp
Outdated
//------------------------------------------------------------------------ | ||
// emitDispScalarReg: Display a the name of a scalar mode of a vector register | ||
// | ||
void emitter::emitDispScalarReg(regNumber reg, insOpts opt, bool addComma) |
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.
pretty sure there is an existing method that does this.
src/coreclr/jit/emitarm64.cpp
Outdated
case IF_SVE_CD_2A: // ........xx...... ......mmmmmddddd -- SVE insert general register | ||
assert(insOptsScalable(id->idInsOpt())); | ||
assert(isVectorRegister(id->idReg1())); // ddddd | ||
assert(isGeneralRegister(id->idReg2())); // mmmmm |
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.
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'm not sure I can assert this one because it is not passed in as an argument, it is derived from the SVE size. But I have also noticed that it is currently accepting Q size which is invalid and I need to fix 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.
i think you should be able to do isValidVectorElemsizeFloat(elemsize)
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.
elemsize
can be any of the standard four B,H,S,D sizes (as shown in the <T>
table). The same size field is then reused for the general purpose register, as W or X, with W taking up 3 of the values (as shown in the <R>
table).
So as long as we assert the B,H,S,D, then the W,X doesn't need checking.
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. Thanks!
Add encodings for the
insr
instruction.Matching capstone output:
Contributing towards #94549