Skip to content

Commit

Permalink
Take the memory operand size for some hw intrinsic instructions (#57603)
Browse files Browse the repository at this point in the history 
* fix to adjust the memory operand size

* Reenable test

* Add under DEBUG

* jit format
  • Loading branch information
@kunalspathak
kunalspathak committed Aug 30, 2021
1 parent d5397f5 commit 7ec2bb0
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 67 deletions.
6 changes: 3 additions & 3 deletions src/coreclr/jit/codegen.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1008,12 +1008,12 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
void genHWIntrinsic(GenTreeHWIntrinsic* node);
#if defined(TARGET_XARCH)
void genHWIntrinsic_R_RM(GenTreeHWIntrinsic* node, instruction ins, emitAttr attr, regNumber reg, GenTree* rmOp);
void genHWIntrinsic_R_RM_I(GenTreeHWIntrinsic* node, instruction ins, int8_t ival);
void genHWIntrinsic_R_RM_I(GenTreeHWIntrinsic* node, instruction ins, emitAttr attr, int8_t ival);
void genHWIntrinsic_R_R_RM(GenTreeHWIntrinsic* node, instruction ins, emitAttr attr);
void genHWIntrinsic_R_R_RM(
GenTreeHWIntrinsic* node, instruction ins, emitAttr attr, regNumber targetReg, regNumber op1Reg, GenTree* op2);
void genHWIntrinsic_R_R_RM_I(GenTreeHWIntrinsic* node, instruction ins, int8_t ival);
void genHWIntrinsic_R_R_RM_R(GenTreeHWIntrinsic* node, instruction ins);
void genHWIntrinsic_R_R_RM_I(GenTreeHWIntrinsic* node, instruction ins, emitAttr attr, int8_t ival);
void genHWIntrinsic_R_R_RM_R(GenTreeHWIntrinsic* node, instruction ins, emitAttr attr);
void genHWIntrinsic_R_R_R_RM(
instruction ins, emitAttr attr, regNumber targetReg, regNumber op1Reg, regNumber op2Reg, GenTree* op3);
void genBaseIntrinsic(GenTreeHWIntrinsic* node);
Expand Down
66 changes: 66 additions & 0 deletions src/coreclr/jit/emit.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1482,6 +1482,10 @@ class emitter
ssize_t emitGetInsCIdisp(instrDesc* id);
unsigned emitGetInsCIargs(instrDesc* id);

#ifdef DEBUG
inline static emitAttr emitGetMemOpSize(instrDesc* id);
#endif // DEBUG

// Return the argument count for a direct call "id".
int emitGetInsCDinfo(instrDesc* id);

Expand Down Expand Up @@ -2793,6 +2797,68 @@ inline unsigned emitter::emitGetInsCIargs(instrDesc* id)
}
}

#ifdef DEBUG
//-----------------------------------------------------------------------------
// emitGetMemOpSize: Get the memory operand size of instrDesc.
//
// Note: vextractf128 has a 128-bit output (register or memory) but a 256-bit input (register).
// vinsertf128 is the inverse with a 256-bit output (register), a 256-bit input(register),
// and a 128-bit input (register or memory).
// This method is mainly used for such instructions to return the appropriate memory operand
// size, otherwise returns the regular operand size of the instruction.

// Arguments:
// id - Instruction descriptor
//
/* static */ emitAttr emitter::emitGetMemOpSize(instrDesc* id)
{
emitAttr defaultSize = id->idOpSize();
emitAttr newSize = defaultSize;
switch (id->idIns())
{
case INS_vextractf128:
case INS_vextracti128:
case INS_vinsertf128:
case INS_vinserti128:
{
return EA_16BYTE;
}

case INS_pextrb:
case INS_pinsrb:
{
return EA_1BYTE;
}

case INS_pextrw:
case INS_pextrw_sse41:
case INS_pinsrw:
{
return EA_2BYTE;
}

case INS_extractps:
case INS_insertps:
case INS_pextrd:
case INS_pinsrd:
{
return EA_4BYTE;
}

case INS_pextrq:
case INS_pinsrq:
{
return EA_8BYTE;
}

default:
{
return id->idOpSize();
}
}
}
#endif // DEBUG

#endif // TARGET_XARCH

/*****************************************************************************
Expand Down
57 changes: 5 additions & 52 deletions src/coreclr/jit/emitxarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8783,58 +8783,9 @@ void emitter::emitDispIns(
}
else
{
emitAttr sizeAttr = id->idOpSize();
attr = sizeAttr;
attr = emitGetMemOpSize(id);

switch (ins)
{
case INS_vextractf128:
case INS_vextracti128:
case INS_vinsertf128:
case INS_vinserti128:
{
sizeAttr = EA_16BYTE;
break;
}

case INS_pextrb:
case INS_pinsrb:
{
sizeAttr = EA_1BYTE;
break;
}

case INS_pextrw:
case INS_pextrw_sse41:
case INS_pinsrw:
{
sizeAttr = EA_2BYTE;
break;
}

case INS_extractps:
case INS_insertps:
case INS_pextrd:
case INS_pinsrd:
{
sizeAttr = EA_4BYTE;
break;
}

case INS_pextrq:
case INS_pinsrq:
{
sizeAttr = EA_8BYTE;
break;
}

default:
{
break;
}
}

sstr = codeGen->genSizeStr(sizeAttr);
sstr = codeGen->genSizeStr(attr);

if (ins == INS_lea)
{
Expand Down Expand Up @@ -11512,7 +11463,8 @@ BYTE* emitter::emitOutputCV(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc)
{
addr = emitConsBlock + doff;

int byteSize = EA_SIZE_IN_BYTES(size);
#ifdef DEBUG
int byteSize = EA_SIZE_IN_BYTES(emitGetMemOpSize(id));

// this instruction has a fixed size (4) src.
if (ins == INS_cvttss2si || ins == INS_cvtss2sd || ins == INS_vbroadcastss)
Expand All @@ -11532,6 +11484,7 @@ BYTE* emitter::emitOutputCV(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc)
assert((emitChkAlign == false) || (ins == INS_lea) ||
((emitComp->compCodeOpt() == Compiler::SMALL_CODE) && (((size_t)addr & 3) == 0)) ||
(((size_t)addr & (byteSize - 1)) == 0));
#endif // DEBUG
}
else
{
Expand Down
20 changes: 9 additions & 11 deletions src/coreclr/jit/hwintrinsiccodegenxarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
instruction ins = HWIntrinsicInfo::lookupIns(intrinsicId, baseType);
assert(ins != INS_invalid);
emitAttr simdSize = emitActualTypeSize(Compiler::getSIMDTypeForSize(node->GetSimdSize()));

assert(simdSize != 0);

switch (numArgs)
Expand Down Expand Up @@ -132,7 +133,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
}
else
{
genHWIntrinsic_R_RM_I(node, ins, static_cast<int8_t>(ival));
genHWIntrinsic_R_RM_I(node, ins, simdSize, static_cast<int8_t>(ival));
}
}
else if ((category == HW_Category_SIMDScalar) && HWIntrinsicInfo::CopiesUpperBits(intrinsicId))
Expand Down Expand Up @@ -201,7 +202,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
if ((ival != -1) && varTypeIsFloating(baseType))
{
assert((ival >= 0) && (ival <= 127));
genHWIntrinsic_R_R_RM_I(node, ins, static_cast<int8_t>(ival));
genHWIntrinsic_R_R_RM_I(node, ins, simdSize, static_cast<int8_t>(ival));
}
else if (category == HW_Category_MemoryLoad)
{
Expand All @@ -226,7 +227,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
else if (HWIntrinsicInfo::isImmOp(intrinsicId, op2))
{
assert(ival == -1);
auto emitSwCase = [&](int8_t i) { genHWIntrinsic_R_RM_I(node, ins, i); };
auto emitSwCase = [&](int8_t i) { genHWIntrinsic_R_RM_I(node, ins, simdSize, i); };

if (op2->IsCnsIntOrI())
{
Expand Down Expand Up @@ -276,7 +277,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
{
assert(ival == -1);

auto emitSwCase = [&](int8_t i) { genHWIntrinsic_R_R_RM_I(node, ins, i); };
auto emitSwCase = [&](int8_t i) { genHWIntrinsic_R_R_RM_I(node, ins, simdSize, i); };

if (op3->IsCnsIntOrI())
{
Expand Down Expand Up @@ -321,7 +322,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
case NI_AVX_BlendVariable:
case NI_AVX2_BlendVariable:
{
genHWIntrinsic_R_R_RM_R(node, ins);
genHWIntrinsic_R_R_RM_R(node, ins, simdSize);
break;
}
case NI_AVXVNNI_MultiplyWideningAndAdd:
Expand Down Expand Up @@ -550,11 +551,10 @@ void CodeGen::genHWIntrinsic_R_RM(
// ins - The instruction being generated
// ival - The immediate value
//
void CodeGen::genHWIntrinsic_R_RM_I(GenTreeHWIntrinsic* node, instruction ins, int8_t ival)
void CodeGen::genHWIntrinsic_R_RM_I(GenTreeHWIntrinsic* node, instruction ins, emitAttr simdSize, int8_t ival)
{
regNumber targetReg = node->GetRegNum();
GenTree* op1 = node->gtGetOp1();
emitAttr simdSize = emitActualTypeSize(Compiler::getSIMDTypeForSize(node->GetSimdSize()));

// TODO-XArch-CQ: Commutative operations can have op1 be contained
// TODO-XArch-CQ: Non-VEX encoded instructions can have both ops contained
Expand Down Expand Up @@ -629,12 +629,11 @@ void CodeGen::genHWIntrinsic_R_R_RM(
// ins - The instruction being generated
// ival - The immediate value
//
void CodeGen::genHWIntrinsic_R_R_RM_I(GenTreeHWIntrinsic* node, instruction ins, int8_t ival)
void CodeGen::genHWIntrinsic_R_R_RM_I(GenTreeHWIntrinsic* node, instruction ins, emitAttr simdSize, int8_t ival)
{
regNumber targetReg = node->GetRegNum();
GenTree* op1 = node->gtGetOp1();
GenTree* op2 = node->gtGetOp2();
emitAttr simdSize = emitActualTypeSize(Compiler::getSIMDTypeForSize(node->GetSimdSize()));
emitter* emit = GetEmitter();

// TODO-XArch-CQ: Commutative operations can have op1 be contained
Expand Down Expand Up @@ -793,13 +792,12 @@ void CodeGen::genHWIntrinsic_R_R_RM_I(GenTreeHWIntrinsic* node, instruction ins,
// node - The hardware intrinsic node
// ins - The instruction being generated
//
void CodeGen::genHWIntrinsic_R_R_RM_R(GenTreeHWIntrinsic* node, instruction ins)
void CodeGen::genHWIntrinsic_R_R_RM_R(GenTreeHWIntrinsic* node, instruction ins, emitAttr simdSize)
{
regNumber targetReg = node->GetRegNum();
GenTree* op1 = node->gtGetOp1();
GenTree* op2 = node->gtGetOp2();
GenTree* op3 = nullptr;
emitAttr simdSize = emitActualTypeSize(Compiler::getSIMDTypeForSize(node->GetSimdSize()));
emitter* emit = GetEmitter();

assert(op1->OperIsList());
Expand Down
1 change: 0 additions & 1 deletion src/tests/Regressions/coreclr/GitHub_34094/Test34094.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<CLRTestKind>BuildAndRun</CLRTestKind>
<CLRTestPriority>0</CLRTestPriority>
<JitOptimizationSensitive>true</JitOptimizationSensitive>
</PropertyGroup>
<ItemGroup>
<Compile Include="test34094.cs" />
Expand Down

0 comments on commit 7ec2bb0

Please sign in to comment.