Skip to content

Add Predicate register placeholder methods and Z registers #95016

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Nov 22, 2023
Merged

Add Predicate register placeholder methods and Z registers #95016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4229b9c
Add placeholder methods for predicate registers
kunalspathak Nov 20, 2023
e2b2dbb
Add Z register display
kunalspathak Nov 20, 2023
8e6403d
Add fake predicate registers
kunalspathak Nov 21, 2023
d547246
Add methods for SVE registers
kunalspathak Nov 22, 2023
05b324d
Add isValidScalableDatasize
kunalspathak Nov 22, 2023
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
140 changes: 139 additions & 1 deletion src/coreclr/jit/emitarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1312,6 +1312,18 @@ static const char * const wRegNames[] =
#include "register.h"
};


static const char * const zRegNames[] =
{
"z0", "z1", "z2", "z3", "z4",
"z5", "z6", "z7", "z8", "z9",
"z10", "z11", "z12", "z13", "z14",
"z15", "z16", "z17", "z18", "z19",
"z20", "z21", "z22", "z23", "z24",
"z25", "z26", "z27", "z28", "z29",
"z30", "z31"
};

static const char * const vRegNames[] =
{
"v0", "v1", "v2", "v3", "v4",
Expand Down Expand Up @@ -1354,6 +1366,14 @@ static const char * const bRegNames[] =
"b25", "b26", "b27", "b28", "b29",
"b30", "b31"
};

static const char * const pRegNames[] =
{
"p0", "p1", "p2", "p3", "p4",
"p5", "p6", "p7", "p8", "p9",
"p10", "p11", "p12", "p13", "p14",
"p15"
};
// clang-format on

//------------------------------------------------------------------------
Expand Down Expand Up @@ -1395,13 +1415,35 @@ const char* emitter::emitRegName(regNumber reg, emitAttr size, bool varName) con
{
rn = bRegNames[reg - REG_V0];
}
else if (size == EA_SCALABLE)
{
rn = zRegNames[reg - REG_V0];
}
}

assert(rn != nullptr);

return rn;
}

//------------------------------------------------------------------------
// emitSveRegName: Returns a scalable vector register name.
//
// Arguments:
// reg - A SIMD and floating-point register.
//
// Return value:
// A string that represents a scalable vector register name.
//
const char* emitter::emitSveRegName(regNumber reg)
{
assert((reg >= REG_V0) && (reg <= REG_V31));

int index = (int)reg - (int)REG_V0;

return zRegNames[index];
}

//------------------------------------------------------------------------
// emitVectorRegName: Returns a SIMD vector register name.
//
Expand All @@ -1420,6 +1462,24 @@ const char* emitter::emitVectorRegName(regNumber reg)
return vRegNames[index];
}

//------------------------------------------------------------------------
// emitPredicateRegName: Returns a predicate register name.
//
// Arguments:
// reg - A predicate register.
//
// Return value:
// A string that represents a predicate register name.
//
const char* emitter::emitPredicateRegName(regNumber reg)
{
assert((reg >= REG_P0) && (reg <= REG_P15));

int index = (int)reg - (int)REG_P0;

return vRegNames[index];
}

/*****************************************************************************
*
* Returns the base encoding of the given CPU instruction.
Expand Down Expand Up @@ -10571,6 +10631,45 @@ void emitter::emitIns_Call(EmitCallType callType,
return ureg << 10;
}

/*****************************************************************************
*
* Returns an encoding for the specified register used in the 'Pd' position
*/

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

/*****************************************************************************
*
* Returns an encoding for the specified register used in the 'Pn' position
*/

/*static*/ emitter::code_t emitter::insEncodeReg_Pn(regNumber reg)
Copy link
Contributor

@a74nh a74nh Nov 20, 2023

Choose a reason for hiding this comment

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

Seems a shame we can't reuse insEncodeReg_Zn etc - the function is very similar.

Copy link
Contributor Author

@kunalspathak kunalspathak Nov 20, 2023

Choose a reason for hiding this comment

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

The only differentiator is the following portion: Could use macro to share most of that code, but don't think it is worth it (for now).

    assert(emitter::isPredicateRegister(reg));
    emitter::code_t ureg = (emitter::code_t)reg - (emitter::code_t)REG_V0;

Copy link
Contributor

@BruceForstall BruceForstall Nov 21, 2023

Choose a reason for hiding this comment

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

I think it's not worth sharing. The asserts will be different even if the code is similar. It'll probably either get inlined or comdat folded in a release build anyway.

{
assert(emitter::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
*/

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

/*****************************************************************************
*
* Returns an encoding for the specified condition code.
Expand Down Expand Up @@ -13768,7 +13867,21 @@ void emitter::emitDispReg(regNumber reg, emitAttr attr, bool addComma)
}

//------------------------------------------------------------------------
// emitDispVectorReg: Display a SIMD vector register name with with an arrangement suffix
// emitDispSveReg: Display a scalable vector register name with an arrangement suffix
//
void emitter::emitDispSveReg(regNumber reg, insOpts opt, bool addComma)
{
assert(insOptsScalable(opt));
assert(isVectorRegister(reg));
printf(emitSveRegName(reg));
emitDispArrangement(opt);

if (addComma)
emitDispComma();
}

//------------------------------------------------------------------------
// emitDispVectorReg: Display a SIMD vector register name with an arrangement suffix
//
void emitter::emitDispVectorReg(regNumber reg, insOpts opt, bool addComma)
{
Expand Down Expand Up @@ -13849,6 +13962,19 @@ void emitter::emitDispVectorElemList(
}
}

//------------------------------------------------------------------------
// emitDispPredicateReg: Display a predicate register name with with an arrangement suffix
//
void emitter::emitDispPredicateReg(regNumber reg, insOpts opt, bool addComma)
{
assert(isPredicateRegister(reg));
printf(emitPredicateRegName(reg));
emitDispArrangement(opt);

if (addComma)
emitDispComma();
}

//------------------------------------------------------------------------
// emitDispArrangement: Display a SIMD vector arrangement suffix
//
Expand All @@ -13864,24 +13990,36 @@ void emitter::emitDispArrangement(insOpts opt)
case INS_OPTS_16B:
str = "16b";
break;
case INS_OPTS_SCALABLE_B:
str = "b";
break;
case INS_OPTS_4H:
str = "4h";
break;
case INS_OPTS_8H:
str = "8h";
break;
case INS_OPTS_SCALABLE_H:
str = "h";
break;
case INS_OPTS_2S:
str = "2s";
break;
case INS_OPTS_4S:
str = "4s";
break;
case INS_OPTS_SCALABLE_S:
str = "s";
break;
case INS_OPTS_1D:
str = "1d";
break;
case INS_OPTS_2D:
str = "2d";
break;
case INS_OPTS_SCALABLE_D:
str = "d";
break;

default:
assert(!"Invalid insOpt for vector register");
Expand Down
29 changes: 29 additions & 0 deletions src/coreclr/jit/emitarm64.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ static bool strictArmAsm;
/* Debug-only routines to display instructions */
/************************************************************************/

const char* emitSveRegName(regNumber reg);
const char* emitVectorRegName(regNumber reg);
const char* emitPredicateRegName(regNumber reg);

void emitDispInsHelp(
instrDesc* id, bool isNew, bool doffs, bool asmfm, unsigned offset, BYTE* pCode, size_t sz, insGroup* ig);
Expand All @@ -38,10 +40,12 @@ void emitDispShiftOpts(insOpts opt);
void emitDispExtendOpts(insOpts opt);
void emitDispLSExtendOpts(insOpts opt);
void emitDispReg(regNumber reg, emitAttr attr, bool addComma);
void emitDispSveReg(regNumber reg, insOpts opt, bool addComma);
void emitDispVectorReg(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);
void emitDispPredicateReg(regNumber reg, insOpts opt, bool addComma);
void emitDispArrangement(insOpts opt);
void emitDispElemsize(emitAttr elemsize);
void emitDispShiftedReg(regNumber reg, insOpts opt, ssize_t imm, emitAttr attr);
Expand Down Expand Up @@ -312,6 +316,15 @@ 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);

// Returns an encoding for the specified register used in the 'Pn' position
static code_t insEncodeReg_Pn(regNumber reg);

// Returns an encoding for the specified register used in the 'Pm' position
static code_t insEncodeReg_Pm(regNumber reg);

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

Expand Down Expand Up @@ -604,6 +617,11 @@ inline static bool isValidScalarDatasize(emitAttr size)
return (size == EA_8BYTE) || (size == EA_4BYTE);
}

inline static bool isValidScalableDatasize(emitAttr size)
{
return ((size & EA_SCALABLE) == EA_SCALABLE);
}

inline static bool isValidVectorDatasize(emitAttr size)
{
return (size == EA_16BYTE) || (size == EA_8BYTE);
Expand Down Expand Up @@ -664,6 +682,11 @@ inline static bool isFloatReg(regNumber reg)
return isVectorRegister(reg);
}

inline static bool isPredicateRegister(regNumber reg)
{
return (reg >= REG_PREDICATE_FIRST && reg <= REG_PREDICATE_LAST);
}

inline static bool insOptsNone(insOpts opt)
{
return (opt == INS_OPTS_NONE);
Expand Down Expand Up @@ -760,6 +783,12 @@ inline static bool insOptsConvertIntToFloat(insOpts opt)
return ((opt >= INS_OPTS_4BYTE_TO_S) && (opt <= INS_OPTS_8BYTE_TO_D));
}

inline static bool insOptsScalable(insOpts opt)
{
return ((opt == INS_OPTS_SCALABLE_B || opt == INS_OPTS_SCALABLE_H || opt == INS_OPTS_SCALABLE_S ||
opt == INS_OPTS_SCALABLE_D));
}

static bool isValidImmCond(ssize_t imm);
static bool isValidImmCondFlags(ssize_t imm);
static bool isValidImmCondFlagsImm5(ssize_t imm);
Expand Down
11 changes: 9 additions & 2 deletions src/coreclr/jit/instr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,11 @@ enum insOpts : unsigned
INS_OPTS_1D,
INS_OPTS_2D,

INS_OPTS_SCALABLE_B,
INS_OPTS_SCALABLE_H,
INS_OPTS_SCALABLE_S,
INS_OPTS_SCALABLE_D,

Copy link
Contributor

@a74nh a74nh Nov 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I did a similar thing in my WIP, but put it inside emitAttr:

#if defined(TARGET_ARM64)
                EA_SIZE_MASK      = 0x01F,
                EA_SCALABLE       = 0x020,
                EA_1BYTE_SCALABLE = EA_1BYTE | EA_SCALABLE,
                EA_2BYTE_SCALABLE = EA_2BYTE | EA_SCALABLE,
                EA_4BYTE_SCALABLE = EA_4BYTE | EA_SCALABLE,
                EA_8BYTE_SCALABLE = EA_8BYTE | EA_SCALABLE,

I'm happy with he way you've done it.

But what do we use for the size?
For a basic AND with integer registers, would you do:

theEmitter->emitIns_R_R_R(INS_sve_and, EA_4BYTE, REG_Z0, REG_P1, REG_Z2, INS_OPTS_SCALABLE_S);

or:

theEmitter->emitIns_R_R_R(INS_sve_and, EA_UNKNOWN, REG_Z0, REG_P1, REG_Z2, INS_OPTS_SCALABLE_S);

?

Copy link
Contributor

@a74nh a74nh Nov 22, 2023

Choose a reason for hiding this comment

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

Never mind, I've just seen EA_SCALABLE. Which gives:

theEmitter->emitIns_R_R_R(INS_sve_and, EA_SCALABLE, REG_Z0, REG_P1, REG_Z2, INS_OPTS_SCALABLE_S);

INS_OPTS_MSL, // Vector Immediate (shifting ones variant)

INS_OPTS_S_TO_4BYTE, // Single to INT32
Expand Down Expand Up @@ -418,8 +423,10 @@ enum emitAttr : unsigned
EA_4BYTE = 0x004,
EA_8BYTE = 0x008,
EA_16BYTE = 0x010,

#if defined(TARGET_XARCH)
#if defined(TARGET_ARM64)
EA_SCALABLE = 0x020,
Copy link
Contributor

@a74nh a74nh Nov 21, 2023

Choose a reason for hiding this comment

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

Intention here is to use scalable alongside another size? For example, for an SVE vector of ints:

    theEmitter->emitIns_R_R_R(INS_sve_and, EA_1BYTE|EA_SCALABLE, REG_V0, REG_V1, REG_V2, INS_OPTS_NONE);  // AND     <Zdn>.<T>, <Pg>/M, <Zdn>.<T>, <Zm>.<T>

Copy link
Contributor Author

@kunalspathak kunalspathak Nov 21, 2023

Choose a reason for hiding this comment

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

I think we need to add the following lane size enum entries in insOpts to represent number of lanes in scalable registers.

INS_OPTS_SCALABLE_B,
INS_OPTS_SCALABLE_H,
INS_OPTS_SCALABLE_S,
INS_OPTS_SCALABLE_D,

Then, for your example, it would be:

   theEmitter->emitIns_R_R_R(INS_sve_and, EA_SCALABLE, REG_V0, REG_V1, REG_V2, INS_OPTS_SCALABLE_B);  // AND     <Zdn>.<T>, <Pg>/M, <Zdn>.<T>, <Zm>.<T>

EA_SIZE_MASK = 0x03F,
#elif defined(TARGET_XARCH)
EA_32BYTE = 0x020,
EA_64BYTE = 0x040,
EA_SIZE_MASK = 0x07F,
Expand Down
20 changes: 20 additions & 0 deletions src/coreclr/jit/registerarm64.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,26 @@ REGDEF(V29, 29+VBASE, VMASK(29), "d29", "s29")
REGDEF(V30, 30+VBASE, VMASK(30), "d30", "s30")
REGDEF(V31, 31+VBASE, VMASK(31), "d31", "s31")

// TODO-SVE: Fix once we add predicate registers
REGALIAS(P0, V0)
REGALIAS(P1, V1)
REGALIAS(P2, V2)
REGALIAS(P3, V3)
REGALIAS(P4, V4)
REGALIAS(P5, V5)
REGALIAS(P6, V6)
REGALIAS(P7, V7)
REGALIAS(P8, V8)
REGALIAS(P9, V9)
REGALIAS(P10, V10)
REGALIAS(P11, V11)
REGALIAS(P12, V12)
REGALIAS(P13, V13)
REGALIAS(P14, V14)
REGALIAS(P15, V15)



// The registers with values 64 (NBASE) and above are not real register numbers
#define NBASE 64

Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/targetarm64.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
#define REG_FP_LAST REG_V31
#define FIRST_FP_ARGREG REG_V0
#define LAST_FP_ARGREG REG_V15
#define REG_PREDICATE_FIRST REG_P0
#define REG_PREDICATE_LAST REG_P15

#define REGNUM_BITS 6 // number of bits in a REG_*
#define REGSIZE_BYTES 8 // number of bytes in one general purpose register
Expand Down