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

Arm64/SVE: Add insEncodeReg* methods #95105

Merged
merged 4 commits into from
Nov 23, 2023
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
217 changes: 206 additions & 11 deletions src/coreclr/jit/emitarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10633,41 +10633,236 @@ void emitter::emitIns_Call(EmitCallType callType,

/*****************************************************************************
*
* Returns an encoding for the specified register used in the 'Pd' position
* Return an encoding for the specified 'V' register used in '4' thru '0' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_Pd(regNumber reg)
/*static*/ emitter::code_t emitter::insEncodeReg_V_4_to_0(regNumber reg)
{
assert(emitter::isPredicateRegister(reg));
assert(isVectorRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_V0;
assert((ureg >= 0) && (ureg <= 32));
return ureg << 0;
}

/*****************************************************************************
*
* Return an encoding for the specified 'V' register used in '9' thru '5' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_V_9_to_5(regNumber reg)
{
assert(isVectorRegister(reg));
Copy link
Contributor

Choose a reason for hiding this comment

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

I want to change this to insEncodeReg_V_9_to_5(regNumber reg, int lowbit, int highbit)
But your way is safer.

At the risk of making more hard to parse macros:

#define insEncodeFunc(highbit, lowbit) emitter::code_t emitter::insEncodeReg_V_#highbit_to_#lowbit(regNumber reg) /
{ /
    emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_V0; /
    assert((ureg >= 0) && (ureg <= 32)); /
    return ureg << 5; /
} /

insEncodeFunc(9, 5);
insEncodeFunc(12, 10);
etc

Copy link
Member Author

Choose a reason for hiding this comment

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

@BruceForstall - any preference? if we don't want to use macro, we can also convert in a function that takes lowbit as a parameter.

Copy link
Contributor

Choose a reason for hiding this comment

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

For some of the predicate ones, I've had to change them to assert(isLowPredicateRegister(reg)); because they only have 4 bits, as opposed to 5 bits.

So, maybe better leaving as functions.

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer functions, even if they are quite duplicative. Makes for easier debugging. Sounds like @a74nh already found reasons the asserts would be different.

emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_V0;
assert((ureg >= 0) && (ureg <= 32));
return ureg << 5;
}

/*****************************************************************************
*
* Return an encoding for the specified 'P' register used in '12' thru '10' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_P_12_to_10(regNumber reg)
{
assert(isPredicateRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_P0;
assert((ureg >= 0) && (ureg <= 15));
return ureg;
return ureg << 10;
}

/*****************************************************************************
*
* Return an encoding for the specified 'V' register used in '21' thru '17' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_V_21_to_17(regNumber reg)
{
assert(isVectorRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_V0;
assert((ureg >= 0) && (ureg <= 32));
return ureg << 17;
}

/*****************************************************************************
*
* Return an encoding for the specified 'R' register used in '21' thru '17' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_R_21_to_17(regNumber reg)
{
assert(isIntegerRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg;
assert((ureg >= 0) && (ureg <= 32));
return ureg << 17;
}

/*****************************************************************************
*
* Return an encoding for the specified 'R' register used in '9' thru '5' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_R_9_to_5(regNumber reg)
{
assert(isIntegerRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg;
assert((ureg >= 0) && (ureg <= 32));
return ureg << 5;
}

/*****************************************************************************
*
* Return an encoding for the specified 'R' register used in '4' thru '0' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_R_4_to_0(regNumber reg)
{
assert(isIntegerRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg;
assert((ureg >= 0) && (ureg <= 32));
return ureg << 0;
}

/*****************************************************************************
*
* Return an encoding for the specified 'P' register used in '20' thru '17' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_P_20_to_17(regNumber reg)
{
assert(isPredicateRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_P0;
assert((ureg >= 0) && (ureg <= 15));
return ureg << 17;
}

/*****************************************************************************
*
* Return an encoding for the specified 'P' register used in '3' thru '0' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_P_3_to_0(regNumber reg)
{
assert(isPredicateRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_P0;
assert((ureg >= 0) && (ureg <= 15));
return ureg << 0;
}

/*****************************************************************************
*
* Returns an encoding for the specified register used in the 'Pn' position
* Return an encoding for the specified 'P' register used in '8' thru '5' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_Pn(regNumber reg)
/*static*/ emitter::code_t emitter::insEncodeReg_P_8_to_5(regNumber reg)
{
assert(emitter::isPredicateRegister(reg));
assert(isPredicateRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_P0;
assert((ureg >= 0) && (ureg <= 15));
return ureg << 5;
}

/*****************************************************************************
*
* Returns an encoding for the specified register used in the 'Pm' position
* Return an encoding for the specified 'P' register used in '13' thru '10' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_Pm(regNumber reg)
/*static*/ emitter::code_t emitter::insEncodeReg_P_13_to_10(regNumber reg)
{
assert(emitter::isPredicateRegister(reg));
assert(isPredicateRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_P0;
assert((ureg >= 0) && (ureg <= 15));
return ureg << 16;
return ureg << 10;
}

/*****************************************************************************
*
* Return an encoding for the specified 'R' register used in '18' thru '17' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_R_18_to_17(regNumber reg)
{
assert(isIntegerRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg;
assert((ureg >= 0) && (ureg <= 32));
return ureg << 17;
}

/*****************************************************************************
*
* Return an encoding for the specified 'P' register used in '7' thru '5' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_P_7_to_5(regNumber reg)
{
assert(isPredicateRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_P0;
assert((ureg >= 0) && (ureg <= 15));
return ureg << 5;
}

/*****************************************************************************
*
* Return an encoding for the specified 'P' register used in '3' thru '1' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_P_3_to_1(regNumber reg)
{
assert(isPredicateRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_P0;
assert((ureg >= 0) && (ureg <= 15));
return ureg << 1;
}

/*****************************************************************************
*
* Return an encoding for the specified 'P' register used in '2' thru '0' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_P_2_to_0(regNumber reg)
{
assert(isPredicateRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_P0;
assert((ureg >= 0) && (ureg <= 15));
return ureg << 0;
}

/*****************************************************************************
*
* Return an encoding for the specified 'V' register used in '19' thru '17' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_V_19_to_17(regNumber reg)
{
assert(isVectorRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_V0;
assert((ureg >= 0) && (ureg <= 32));
return ureg << 17;
}

/*****************************************************************************
*
* Return an encoding for the specified 'V' register used in '20' thru '17' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_V_20_to_17(regNumber reg)
{
assert(isVectorRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_V0;
assert((ureg >= 0) && (ureg <= 32));
return ureg << 17;
}

/*****************************************************************************
*
* Return an encoding for the specified 'V' register used in '9' thru '6' position.
*/

/*static*/ emitter::code_t emitter::insEncodeReg_V_9_to_6(regNumber reg)
{
assert(isVectorRegister(reg));
emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_V0;
assert((ureg >= 0) && (ureg <= 32));
return ureg << 6;
}

/*****************************************************************************
Expand Down
57 changes: 51 additions & 6 deletions src/coreclr/jit/emitarm64.h
Original file line number Diff line number Diff line change
Expand Up @@ -316,14 +316,59 @@ static code_t insEncodeReg_Vm(regNumber reg);
// Returns an encoding for the specified register used in the 'Va' position
static code_t insEncodeReg_Va(regNumber reg);

// Returns an encoding for the specified register used in the 'Pd' position
static code_t insEncodeReg_Pd(regNumber reg);
// Return an encoding for the specified register used in '4' thru '0' position.
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
static code_t insEncodeReg_V_4_to_0(regNumber reg);

// Returns an encoding for the specified register used in the 'Pn' position
static code_t insEncodeReg_Pn(regNumber reg);
// Return an encoding for the specified register used in '9' thru '5' position.
static code_t insEncodeReg_V_9_to_5(regNumber reg);

// Returns an encoding for the specified register used in the 'Pm' position
static code_t insEncodeReg_Pm(regNumber reg);
// Return an encoding for the specified register used in '12' thru '10' position.
static code_t insEncodeReg_P_12_to_10(regNumber reg);

// Return an encoding for the specified register used in '21' thru '17' position.
static code_t insEncodeReg_V_21_to_17(regNumber reg);

// Return an encoding for the specified register used in '21' thru '17' position.
static code_t insEncodeReg_R_21_to_17(regNumber reg);

// Return an encoding for the specified register used in '9' thru '5' position.
static code_t insEncodeReg_R_9_to_5(regNumber reg);

// Return an encoding for the specified register used in '4' thru '0' position.
static code_t insEncodeReg_R_4_to_0(regNumber reg);

// Return an encoding for the specified register used in '20' thru '17' position.
static code_t insEncodeReg_P_20_to_17(regNumber reg);

// Return an encoding for the specified register used in '3' thru '0' position.
static code_t insEncodeReg_P_3_to_0(regNumber reg);

// Return an encoding for the specified register used in '8' thru '5' position.
static code_t insEncodeReg_P_8_to_5(regNumber reg);

// Return an encoding for the specified register used in '13' thru '10' position.
static code_t insEncodeReg_P_13_to_10(regNumber reg);

// Return an encoding for the specified register used in '18' thru '17' position.
static code_t insEncodeReg_R_18_to_17(regNumber reg);

// Return an encoding for the specified register used in '7' thru '5' position.
static code_t insEncodeReg_P_7_to_5(regNumber reg);

// Return an encoding for the specified register used in '3' thru '1' position.
static code_t insEncodeReg_P_3_to_1(regNumber reg);

// Return an encoding for the specified register used in '2' thru '0' position.
static code_t insEncodeReg_P_2_to_0(regNumber reg);

// Return an encoding for the specified register used in '19' thru '17' position.
static code_t insEncodeReg_V_19_to_17(regNumber reg);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should all the V ones be Z?

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't introduced any "Z" terminology except for displaying the disassembly so kept it as insEncodeReg_V*. Even we use REG_V0, etc. Perhaps I should add aliases for "Z" registers and use it as well as rename methods whereever necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

My preference would be Z, and introduce REG_Z0 etc to make the code easier to read.
Same with REG_P0.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done for #95129


// Return an encoding for the specified register used in '20' thru '17' position.
static code_t insEncodeReg_V_20_to_17(regNumber reg);

// Return an encoding for the specified register used in '9' thru '6' position.
static code_t insEncodeReg_V_9_to_6(regNumber reg);
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, you already have 9_to_5.
Confused as to exactly what this one is, as it is only 4 bits.

Copy link
Member Author

Choose a reason for hiding this comment

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

If I search for insEncodeReg_V_9_to_6 in emitOutputInstr_sve.cpp, I see this:

case IF_SVE_HG_2A:   // ................ ......nnnn.ddddd -- SVE2 FP8 downconverts
   code = emitInsCodeSve(ins, fmt);
   code |= insEncodeReg_V_4_to_0(id->idReg10()); // ddddd
   code |= insEncodeReg_V_9_to_6(id->idReg20()); // nnnn
   dst += emitOutput_Instr(dst, code);
   break;
case IF_SVE_GA_2A:   // ............iiii ......nnnn.ddddd -- SME2 multi-vec shift narrow
   code = emitInsCodeSve(ins, fmt);
   code |= insEncodeReg_V_4_to_0(id->idReg10()); // ddddd
   code |= insEncodeReg_V_9_to_6(id->idReg20()); // nnnn
   code |= insEncodeImm(); // iiii
   dst += emitOutput_Instr(dst, code);
   break;

Then, searching for IF_SVE_HG_2A and IF_SVE_GA_2A in instrsarm64_sve.h`, I see this:

//    enum               name                     info                                              SVE_HG_2A                                    
INST1(bfcvtn,            "bfcvtn",                0,                       IF_SVE_HG_2A,            0x650A3800                                   )
    // BFCVTN  <Zd>.B, {<Zn1>.H-<Zn2>.H }                                                SVE_HG_2A           0110010100001010 001110nnnn0ddddd     650A 3800   

//    enum               name                     info                                              SVE_GA_2A                                    
INST1(sqrshrn,           "sqrshrn",               RSH,                     IF_SVE_GA_2A,            0x45B02800                                   )
    // SQRSHRN <Zd>.H, {<Zn1>.S-<Zn2>.S }, #<const>                                      SVE_GA_2A           010001011011iiii 001010nnnn0ddddd     45B0 2800  
image image

Most likely the SVE2 instructions takes only 4 bits for Z registers. Also it seems that https://docsmirror.github.io/A64/2023-06/sveindex.html doesn't have SVE2 instructions. I didn't find an entry of "bfcvtn" in there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, looks like this is similar to the instructions that only allow predicate registers 0 to 7.


// Returns an encoding for the imm which represents the condition code.
static code_t insEncodeCond(insCond cond);
Expand Down