Skip to content

Commit

Permalink
Optimize Vector64 and Vector128.Create methods (#36267)
Browse files Browse the repository at this point in the history 
* Updating the Vector64 and Vector128 Create methods to be marked Intrinsic on ARM64

* Updating the JIT to emit constants for Vector64 and Vector128.Create

* Fixing lookupNamedIntrinsic and impIntrinsic to throw PNSE for unsupported mustExpand intrinsics

* Fixing impIntrinsic to directly use gtNewMustThrowException

* Move gtNewMustThrowException to not depend on FEATURE_HW_INTRINSICS

* Applying formatting patch

* Add basic support for GT_CLS_VAR_ADDR to the ARM64 JIT

* Update lookupNamedIntrinsic to handle System.Runtime.Intrinsics for unsupported platforms

* Fixing INS_ldr in emitIns_R_C to use isValidVectorLSDatasize

* Elaborate on why we specially recognize the HWIntrinsics even on platforms that don't support them
  • Loading branch information
@tannergooding
tannergooding authored May 13, 2020
1 parent b221687 commit 9924705
Show file tree
Hide file tree
Showing 16 changed files with 898 additions and 533 deletions.
11 changes: 6 additions & 5 deletions src/coreclr/src/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2657,14 +2657,15 @@ class Compiler
NamedIntrinsic hwIntrinsicID);
GenTreeHWIntrinsic* gtNewScalarHWIntrinsicNode(
var_types type, GenTree* op1, GenTree* op2, GenTree* op3, NamedIntrinsic hwIntrinsicID);
GenTree* gtNewMustThrowException(unsigned helper, var_types type, CORINFO_CLASS_HANDLE clsHnd);
CORINFO_CLASS_HANDLE gtGetStructHandleForHWSIMD(var_types simdType, var_types simdBaseType);
var_types getBaseTypeFromArgIfNeeded(NamedIntrinsic intrinsic,
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_SIG_INFO* sig,
var_types baseType);
#endif // FEATURE_HW_INTRINSICS

GenTree* gtNewMustThrowException(unsigned helper, var_types type, CORINFO_CLASS_HANDLE clsHnd);

GenTreeLclFld* gtNewLclFldNode(unsigned lnum, var_types type, unsigned offset);
GenTree* gtNewInlineCandidateReturnExpr(GenTree* inlineCandidate, var_types type);

Expand Down Expand Up @@ -3713,17 +3714,17 @@ class Compiler
CorInfoIntrinsics intrinsicID,
bool tailCall);
NamedIntrinsic lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method);
GenTree* impUnsupportedNamedIntrinsic(unsigned helper,
CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig,
bool mustExpand);

#ifdef FEATURE_HW_INTRINSICS
GenTree* impHWIntrinsic(NamedIntrinsic intrinsic,
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig,
bool mustExpand);
GenTree* impUnsupportedHWIntrinsic(unsigned helper,
CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig,
bool mustExpand);
GenTree* impSimdAsHWIntrinsic(NamedIntrinsic intrinsic,
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_METHOD_HANDLE method,
Expand Down
13 changes: 10 additions & 3 deletions src/coreclr/src/jit/emitarm64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7548,7 +7548,7 @@ void emitter::emitIns_R_C(
fmt = IF_LARGELDC;
if (isVectorRegister(reg))
{
assert(isValidScalarDatasize(size));
assert(isValidVectorLSDatasize(size));
// For vector (float/double) register, we should have an integer address reg to
// compute long address which consists of page address and page offset.
// For integer constant, this is not needed since the dest reg can be used to
Expand All @@ -7561,6 +7561,7 @@ void emitter::emitIns_R_C(
assert(isValidGeneralDatasize(size));
}
break;

default:
unreached();
}
Expand Down Expand Up @@ -12718,7 +12719,7 @@ void emitter::emitInsLoadStoreOp(instruction ins, emitAttr attr, regNumber dataR

if (addr->isContained())
{
assert(addr->OperGet() == GT_LCL_VAR_ADDR || addr->OperGet() == GT_LEA);
assert(addr->OperGet() == GT_CLS_VAR_ADDR || addr->OperGet() == GT_LCL_VAR_ADDR || addr->OperGet() == GT_LEA);

int offset = 0;
DWORD lsl = 0;
Expand Down Expand Up @@ -12795,7 +12796,13 @@ void emitter::emitInsLoadStoreOp(instruction ins, emitAttr attr, regNumber dataR
}
else // no Index register
{
if (emitIns_valid_imm_for_ldst_offset(offset, emitTypeSize(indir->TypeGet())))
if (addr->OperGet() == GT_CLS_VAR_ADDR)
{
// Get a temp integer register to compute long address.
regNumber addrReg = indir->GetSingleTempReg();
emitIns_R_C(ins, attr, dataReg, addrReg, addr->AsClsVar()->gtClsVarHnd, 0);
}
else if (emitIns_valid_imm_for_ldst_offset(offset, emitTypeSize(indir->TypeGet())))
{
// Then load/store dataReg from/to [memBase + offset]
emitIns_R_R_I(ins, attr, dataReg, memBase->GetRegNum(), offset);
Expand Down
68 changes: 34 additions & 34 deletions src/coreclr/src/jit/gentree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18659,40 +18659,6 @@ GenTreeHWIntrinsic* Compiler::gtNewScalarHWIntrinsicNode(
GenTreeHWIntrinsic(type, gtNewArgList(op1, op2, op3), hwIntrinsicID, TYP_UNKNOWN, 0);
}

//---------------------------------------------------------------------------------------
// gtNewMustThrowException:
// create a throw node (calling into JIT helper) that must be thrown.
// The result would be a comma node: COMMA(jithelperthrow(void), x) where x's type should be specified.
//
// Arguments
// helper - JIT helper ID
// type - return type of the node
//
// Return Value
// pointer to the throw node
//
GenTree* Compiler::gtNewMustThrowException(unsigned helper, var_types type, CORINFO_CLASS_HANDLE clsHnd)
{
GenTreeCall* node = gtNewHelperCallNode(helper, TYP_VOID);
node->gtCallMoreFlags |= GTF_CALL_M_DOES_NOT_RETURN;
if (type != TYP_VOID)
{
unsigned dummyTemp = lvaGrabTemp(true DEBUGARG("dummy temp of must thrown exception"));
if (type == TYP_STRUCT)
{
lvaSetStruct(dummyTemp, clsHnd, false);
type = lvaTable[dummyTemp].lvType; // struct type is normalized
}
else
{
lvaTable[dummyTemp].lvType = type;
}
GenTree* dummyNode = gtNewLclvNode(dummyTemp, type);
return gtNewOperNode(GT_COMMA, type, node, dummyNode);
}
return node;
}

// Returns true for the HW Instrinsic instructions that have MemoryLoad semantics, false otherwise
bool GenTreeHWIntrinsic::OperIsMemoryLoad() const
{
Expand Down Expand Up @@ -18782,6 +18748,40 @@ bool GenTreeHWIntrinsic::OperIsMemoryLoadOrStore() const

#endif // FEATURE_HW_INTRINSICS

//---------------------------------------------------------------------------------------
// gtNewMustThrowException:
// create a throw node (calling into JIT helper) that must be thrown.
// The result would be a comma node: COMMA(jithelperthrow(void), x) where x's type should be specified.
//
// Arguments
// helper - JIT helper ID
// type - return type of the node
//
// Return Value
// pointer to the throw node
//
GenTree* Compiler::gtNewMustThrowException(unsigned helper, var_types type, CORINFO_CLASS_HANDLE clsHnd)
{
GenTreeCall* node = gtNewHelperCallNode(helper, TYP_VOID);
node->gtCallMoreFlags |= GTF_CALL_M_DOES_NOT_RETURN;
if (type != TYP_VOID)
{
unsigned dummyTemp = lvaGrabTemp(true DEBUGARG("dummy temp of must thrown exception"));
if (type == TYP_STRUCT)
{
lvaSetStruct(dummyTemp, clsHnd, false);
type = lvaTable[dummyTemp].lvType; // struct type is normalized
}
else
{
lvaTable[dummyTemp].lvType = type;
}
GenTree* dummyNode = gtNewLclvNode(dummyTemp, type);
return gtNewOperNode(GT_COMMA, type, node, dummyNode);
}
return node;
}

//---------------------------------------------------------------------------------------
// InitializeStructReturnType:
// Initialize the Return Type Descriptor for a method that returns a struct type
Expand Down
44 changes: 0 additions & 44 deletions src/coreclr/src/jit/hwintrinsic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,50 +89,6 @@ var_types Compiler::getBaseTypeFromArgIfNeeded(NamedIntrinsic intrinsic,
return baseType;
}

//------------------------------------------------------------------------
// impUnsupportedHWIntrinsic: returns a node for an unsupported HWIntrinsic
//
// Arguments:
// helper - JIT helper ID for the exception to be thrown
// method - method handle of the intrinsic function.
// sig - signature of the intrinsic call
// mustExpand - true if the intrinsic must return a GenTree*; otherwise, false
//
// Return Value:
// a gtNewMustThrowException if mustExpand is true; otherwise, nullptr
//
GenTree* Compiler::impUnsupportedHWIntrinsic(unsigned helper,
CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig,
bool mustExpand)
{
// We've hit some error case and may need to return a node for the given error.
//
// When `mustExpand=false`, we are attempting to inline the intrinsic directly into another method. In this
// scenario, we need to return `nullptr` so that a GT_CALL to the intrinsic is emitted instead. This is to
// ensure that everything continues to behave correctly when optimizations are enabled (e.g. things like the
// inliner may expect the node we return to have a certain signature, and the `MustThrowException` node won't
// match that).
//
// When `mustExpand=true`, we are in a GT_CALL to the intrinsic and are attempting to JIT it. This will generally
// be in response to an indirect call (e.g. done via reflection) or in response to an earlier attempt returning
// `nullptr` (under `mustExpand=false`). In that scenario, we are safe to return the `MustThrowException` node.

if (mustExpand)
{
for (unsigned i = 0; i < sig->numArgs; i++)
{
impPopStack();
}

return gtNewMustThrowException(helper, JITtype2varType(sig->retType), sig->retTypeClass);
}
else
{
return nullptr;
}
}

CORINFO_CLASS_HANDLE Compiler::gtGetStructHandleForHWSIMD(var_types simdType, var_types simdBaseType)
{
if (simdType == TYP_SIMD16)
Expand Down
41 changes: 41 additions & 0 deletions src/coreclr/src/jit/hwintrinsicarm64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,47 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
assert(retNode->gtType == getSIMDTypeForSize(getSIMDTypeSizeInBytes(sig->retTypeSigClass)));
break;
}

case NI_Vector64_Create:
case NI_Vector128_Create:
{
// We shouldn't handle this as an intrinsic if the
// respective ISAs have been disabled by the user.

if (!compExactlyDependsOn(InstructionSet_AdvSimd))
{
break;
}

if (sig->numArgs == 1)
{
op1 = impPopStack().val;
retNode = gtNewSimdHWIntrinsicNode(retType, op1, intrinsic, baseType, simdSize);
}
else if (sig->numArgs == 2)
{
op2 = impPopStack().val;
op1 = impPopStack().val;
retNode = gtNewSimdHWIntrinsicNode(retType, op1, op2, intrinsic, baseType, simdSize);
}
else
{
assert(sig->numArgs >= 3);

GenTreeArgList* tmp = nullptr;

for (unsigned i = 0; i < sig->numArgs; i++)
{
tmp = gtNewArgList(impPopStack().val);
tmp->gtOp2 = op1;
op1 = tmp;
}

retNode = gtNewSimdHWIntrinsicNode(retType, op1, intrinsic, baseType, simdSize);
}
break;
}

case NI_Vector64_get_Count:
case NI_Vector128_get_Count:
{
Expand Down
8 changes: 6 additions & 2 deletions src/coreclr/src/jit/hwintrinsiccodegenarm64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,10 +583,9 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
GetEmitter()->emitIns_R_I(ins, emitSize, targetReg, 0, INS_OPTS_4S);
break;

case NI_Vector64_Create:
case NI_Vector128_Create:
case NI_AdvSimd_DuplicateToVector64:
case NI_AdvSimd_DuplicateToVector128:
case NI_AdvSimd_Arm64_DuplicateToVector64:
case NI_AdvSimd_Arm64_DuplicateToVector128:
{
if (varTypeIsFloating(intrin.baseType))
Expand All @@ -596,6 +595,11 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
const double dataValue = intrin.op1->AsDblCon()->gtDconVal;
GetEmitter()->emitIns_R_F(INS_fmov, emitSize, targetReg, dataValue, opt);
}
else if (intrin.id == NI_AdvSimd_Arm64_DuplicateToVector64)
{
assert(intrin.baseType == TYP_DOUBLE);
GetEmitter()->emitIns_R_R(ins, emitSize, targetReg, op1Reg, opt);
}
else
{
GetEmitter()->emitIns_R_R_I(ins, emitSize, targetReg, op1Reg, 0, opt);
Expand Down
Loading

0 comments on commit 9924705

Please sign in to comment.