Skip to content
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

Merged
merged 5 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/coreclr/jit/codegenarm64test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5912,6 +5912,26 @@ void CodeGen::genArm64EmitterUnitTestsSve()
theEmitter->emitIns_R_PATTERN_I(INS_sve_incw, EA_SCALABLE, REG_V5, SVE_PATTERN_VL6, 16,
INS_OPTS_SCALABLE_S); // INCW <Zdn>.S{, <pattern>{, MUL #<imm>}}

// IF_SVE_CC_2A
theEmitter->emitIns_R_R(INS_sve_insr, EA_SCALABLE, REG_V0, REG_V13,
INS_OPTS_SCALABLE_B); // INSR <Zdn>.<T>, <V><m>
theEmitter->emitIns_R_R(INS_sve_insr, EA_SCALABLE, REG_V29, REG_V0,
INS_OPTS_SCALABLE_H); // INSR <Zdn>.<T>, <V><m>
theEmitter->emitIns_R_R(INS_sve_insr, EA_SCALABLE, REG_V4, REG_V15,
INS_OPTS_SCALABLE_S); // INSR <Zdn>.<T>, <V><m>
theEmitter->emitIns_R_R(INS_sve_insr, EA_SCALABLE, REG_V8, REG_V2,
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,
Copy link
Member

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?

INS_OPTS_SCALABLE_B); // INSR <Zdn>.<T>, <R><m>
theEmitter->emitIns_R_R(INS_sve_insr, EA_SCALABLE, REG_V11, REG_R1,
INS_OPTS_SCALABLE_H); // INSR <Zdn>.<T>, <R><m>
theEmitter->emitIns_R_R(INS_sve_insr, EA_SCALABLE, REG_V14, REG_R9,
INS_OPTS_SCALABLE_S); // INSR <Zdn>.<T>, <R><m>
theEmitter->emitIns_R_R(INS_sve_insr, EA_SCALABLE, REG_V19, REG_R0,
INS_OPTS_SCALABLE_D); // INSR <Zdn>.<T>, <R><m>

// IF_SVE_CI_3A
theEmitter->emitIns_R_R_R(INS_sve_trn1, EA_SCALABLE, REG_P1, REG_P3, REG_P4,
INS_OPTS_SCALABLE_B); // TRN1 <Pd>.<T>, <Pn>.<T>, <Pm>.<T>
Expand Down
127 changes: 127 additions & 0 deletions src/coreclr/jit/emitarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1210,6 +1210,18 @@ void emitter::emitInsSanityCheck(instrDesc* id)
assert(isValidUimm<2>(emitGetInsSC(id))); // ii
break;

case IF_SVE_CC_2A: // ........xx...... ......mmmmmddddd -- SVE insert SIMD&FP scalar register
assert(insOptsScalable(id->idInsOpt()));
assert(isVectorRegister(id->idReg1())); // ddddd
assert(isVectorRegister(id->idReg2())); // mmmmm
break;

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert(isGeneralRegister(id->idReg2())); // mmmmm
assert(isGeneralRegisterOrZR(id->idReg2())); // mmmmm

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For SVE_CD_2A , we should also assert the "width specifier" ( field)

image

Copy link
Contributor Author

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.

Copy link
Member

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)

Copy link
Contributor

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.

break;

case IF_SVE_CI_3A: // ........xx..MMMM .......NNNN.DDDD -- SVE permute predicate elements
elemsize = id->idOpSize();
assert(insOptsScalableStandard(id->idInsOpt()));
Expand Down Expand Up @@ -3244,6 +3256,28 @@ static const char * const qRegNames[] =
"q30", "q31"
};

static const char * const dRegNames[] =
Copy link
Member

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

REGDEF(V0, 0+VBASE, VMASK(0), "d0", "s0")

{
"d0", "d1", "d2", "d3", "d4",
"d5", "d6", "d7", "d8", "d9",
"d10", "d11", "d12", "d13", "d14",
"d15", "d16", "d17", "d18", "d19",
"d20", "d21", "d22", "d23", "d24",
"d25", "d26", "d27", "d28", "d29",
"d30", "d31"
};

static const char * const sRegNames[] =
{
"s0", "s1", "s2", "s3", "s4",
"s5", "s6", "s7", "s8", "s9",
"s10", "s11", "s12", "s13", "s14",
"s15", "s16", "s17", "s18", "s19",
"s20", "s21", "s22", "s23", "s24",
"s25", "s26", "s27", "s28", "s29",
"s30", "s31"
};

static const char * const hRegNames[] =
{
"h0", "h1", "h2", "h3", "h4",
Expand Down Expand Up @@ -3381,6 +3415,36 @@ const char* emitter::emitVectorRegName(regNumber reg)
return vRegNames[index];
}

//------------------------------------------------------------------------
// emitSimdScalarRegName: Returns a SIMD scalar register name.
//
// Arguments:
// reg - A SIMD and floating-point register.
//
// Return value:
// A string that represents a SIMD scalar register name.
//
const char* emitter::emitSimdScalarRegName(regNumber reg, emitAttr attr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
assert(isVectorRegister(reg));
int index = (int)reg - (int)REG_V0;

switch (attr)
{
case EA_1BYTE:
return bRegNames[index];
case EA_2BYTE:
return hRegNames[index];
case EA_4BYTE:
return sRegNames[index];
case EA_8BYTE:
return dRegNames[index];
default:
unreached();
break;
}
}

//------------------------------------------------------------------------
// emitPredicateRegName: Returns a predicate register name.
//
Expand Down Expand Up @@ -9004,6 +9068,23 @@ void emitter::emitIns_R_R(instruction ins,
break;
}

case INS_sve_insr:
assert(insOptsScalable(opt));
assert(isVectorRegister(reg1)); // ddddd
if (isVectorRegister(reg2))
{
fmt = IF_SVE_CC_2A;
}
else if (isGeneralRegister(reg2))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
else if (isGeneralRegister(reg2))
else if (isGeneralRegisterOrZR(reg2))

{
fmt = IF_SVE_CD_2A;
}
else
{
unreached();
}
break;

case INS_sve_pfirst:
assert(opt == INS_OPTS_SCALABLE_B);
assert(isPredicateRegister(reg1)); // DDDD
Expand Down Expand Up @@ -24488,6 +24569,22 @@ BYTE* emitter::emitOutput_InstrSve(BYTE* dst, instrDesc* id)
dst += emitOutput_Instr(dst, code);
break;

case IF_SVE_CC_2A: // ........xx...... ......mmmmmddddd -- SVE insert SIMD&FP scalar register
code = emitInsCodeSve(ins, fmt);
code |= insEncodeReg_V_4_to_0(id->idReg1()); // ddddd
code |= insEncodeReg_V_9_to_5(id->idReg2()); // mmmmm
code |= insEncodeSveElemsize(optGetSveElemsize(id->idInsOpt())); // xx
dst += emitOutput_Instr(dst, code);
break;

case IF_SVE_CD_2A: // ........xx...... ......mmmmmddddd -- SVE insert general register
code = emitInsCodeSve(ins, fmt);
code |= insEncodeReg_V_4_to_0(id->idReg1()); // ddddd
code |= insEncodeReg_R_9_to_5(id->idReg2()); // mmmmm
code |= insEncodeSveElemsize(optGetSveElemsize(id->idInsOpt())); // xx
dst += emitOutput_Instr(dst, code);
break;

case IF_SVE_CI_3A: // ........xx..MMMM .......NNNN.DDDD -- SVE permute predicate elements
code = emitInsCodeSve(ins, fmt);
code |= insEncodeReg_P_3_to_0(id->idReg1()); // DDDD
Expand Down Expand Up @@ -26491,6 +26588,18 @@ void emitter::emitDispSveRegIndex(regNumber reg, ssize_t index, bool addComma)
emitDispElementIndex(index, addComma);
}

//------------------------------------------------------------------------
// emitDispScalarReg: Display a the name of a scalar mode of a vector register
//
void emitter::emitDispScalarReg(regNumber reg, insOpts opt, bool addComma)
Copy link
Member

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.

{
assert(isVectorRegister(reg));
printf(emitSimdScalarRegName(reg, optGetSveElemsize(opt)));

if (addComma)
emitDispComma();
}

//------------------------------------------------------------------------
// emitDispVectorReg: Display a SIMD vector register name with an arrangement suffix
//
Expand Down Expand Up @@ -28673,6 +28782,18 @@ void emitter::emitDispInsHelp(
printf("]");
break;

// <Zdn>.<T>, <V><m>
case IF_SVE_CC_2A: // ........xx...... ......mmmmmddddd -- SVE insert SIMD&FP scalar register
emitDispSveReg(id->idReg1(), id->idInsOpt(), true); // ddddd
emitDispScalarReg(id->idReg2(), id->idInsOpt(), false); // mmmmm
break;

// <Zdn>.<T>, <R><m>
case IF_SVE_CD_2A: // ........xx...... ......mmmmmddddd -- SVE insert general register
emitDispSveReg(id->idReg1(), id->idInsOpt(), true); // ddddd
emitDispReg(id->idReg2(), id->idInsOpt() == INS_OPTS_SCALABLE_D ? EA_8BYTE : EA_4BYTE, false); // mmmmm
break;

// <Pd>.H, <Pn>.B
case IF_SVE_CK_2A: // ................ .......NNNN.DDDD -- SVE unpack predicate elements
emitDispPredicateReg(id->idReg1(), insGetPredicateType(fmt), INS_OPTS_SCALABLE_H, true); // DDDD
Expand Down Expand Up @@ -32642,6 +32763,12 @@ emitter::insExecutionCharacteristics emitter::getInsExecutionCharacteristics(ins
result.insLatency = PERFSCORE_LATENCY_140C;
break;

case IF_SVE_CC_2A: // ........xx...... ......mmmmmddddd -- SVE insert SIMD&FP scalar register
case IF_SVE_CD_2A: // ........xx...... ......mmmmmddddd -- SVE insert general register
result.insThroughput = PERFSCORE_THROUGHPUT_1C;
result.insLatency = PERFSCORE_LATENCY_5C;
break;

case IF_SVE_CI_3A: // ........xx..MMMM .......NNNN.DDDD -- SVE permute predicate elements
case IF_SVE_CJ_2A: // ........xx...... .......NNNN.DDDD -- SVE reverse predicate elements
case IF_SVE_CK_2A: // ................ .......NNNN.DDDD -- SVE unpack predicate elements
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/emitarm64.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ enum PredicateType

const char* emitSveRegName(regNumber reg);
const char* emitVectorRegName(regNumber reg);
const char* emitSimdScalarRegName(regNumber reg, emitAttr attr);
const char* emitPredicateRegName(regNumber reg, PredicateType ptype);

void emitDispInsHelp(
Expand Down Expand Up @@ -62,6 +63,7 @@ void emitDispSveReg(regNumber reg, bool addComma);
void emitDispSveReg(regNumber reg, insOpts opt, bool addComma);
void emitDispSveRegIndex(regNumber reg, ssize_t index, bool addComma);
void emitDispVectorReg(regNumber reg, insOpts opt, bool addComma);
void emitDispScalarReg(regNumber reg, insOpts opt, bool addComma);
void emitDispVectorRegIndex(regNumber reg, emitAttr elemsize, ssize_t index, bool addComma);
void emitDispVectorRegList(regNumber firstReg, unsigned listSize, insOpts opt, bool addComma);
void emitDispVectorElemList(regNumber firstReg, unsigned listSize, emitAttr elemsize, unsigned index, bool addComma);
Expand Down
Loading