From 99fb6235a6f09adff5b4dc864a4420338016d660 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 18 Jul 2024 11:18:37 +0200 Subject: [PATCH 1/5] Expand nullable unbox in JIT --- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/importer.cpp | 94 ++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index e7822dc591597b..f8b500dac2c235 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4514,6 +4514,7 @@ class Compiler GenTree* impStoreNullableFields(CORINFO_CLASS_HANDLE nullableCls, GenTree* value); + GenTree* impUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenTree* obj); void impLoadNullableFields(GenTree* nullableObj, CORINFO_CLASS_HANDLE nullableCls, GenTree** hasValueFld, GenTree** valueFld); int impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 6b4323de0c5491..cbac04e1965736 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -2832,6 +2832,95 @@ GenTree* Compiler::impImportLdvirtftn(GenTree* thisPtr, return call; } +//------------------------------------------------------------------------ +// impUnboxNullable: Generate code for unboxing Nullable from an object (obj) +// We're going to emit the following IR: +// +// Nullable result; +// if (obj == null) +// { +// result = default; +// } +// else if (obj->pMT == ) +// { +// result._hasValue = true; +// result._value = *(T*)(obj + sizeof(void*)); +// } +// else +// { +// result = CORINFO_HELP_UNBOX_NULLABLE(&result, nullableCls, obj); +// } +// +// Arguments: +// nullableCls - class handle representing the Nullable type +// obj - object to unbox +// +// Return Value: +// A local node representing the unboxed value (Nullable) +// +GenTree* Compiler::impUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenTree* obj) +{ + assert(info.compCompHnd->isNullableType(nullableCls) == TypeCompareState::Must); + + unsigned resultTmp = lvaGrabTemp(true DEBUGARG("Nullable tmp")); + lvaSetStruct(resultTmp, nullableCls, false); + lvaGetDesc(resultTmp)->lvHasLdAddrOp = true; + + // The underlying type of the nullable: + CORINFO_CLASS_HANDLE unboxType = info.compCompHnd->getTypeForBox(nullableCls); + + // Clone the object (and spill side effects) + GenTree* objClone; + obj = impCloneExpr(obj, &objClone, CHECK_SPILL_ALL, nullptr DEBUGARG("op1 spilled for Nullable unbox")); + + // Unbox the object to the result local: + // + // result._hasValue = true; + // result._value = MethodTableLookup(obj); + // + CORINFO_FIELD_HANDLE valueFldHnd = info.compCompHnd->getFieldInClass(nullableCls, 1); + CORINFO_CLASS_HANDLE valueStructCls; + var_types valueType = JITtype2varType(info.compCompHnd->getFieldType(valueFldHnd, &valueStructCls)); + static_assert_no_msg(OFFSETOF__CORINFO_NullableOfT__hasValue == 0); + unsigned hasValOffset = OFFSETOF__CORINFO_NullableOfT__hasValue; + unsigned valueOffset = info.compCompHnd->getFieldOffset(valueFldHnd); + + GenTree* boxedContentAddr = + gtNewOperNode(GT_ADD, TYP_BYREF, gtCloneExpr(objClone), gtNewIconNode(TARGET_POINTER_SIZE, TYP_I_IMPL)); + // Load the boxed content from the object (op1): + GenTree* boxedContent = valueType == TYP_STRUCT ? gtNewBlkIndir(typGetObjLayout(valueStructCls), boxedContentAddr) + : gtNewIndir(valueType, boxedContentAddr); + // Now do two stores via a comma: + GenTree* setHasValue = gtNewStoreLclFldNode(resultTmp, TYP_UBYTE, hasValOffset, gtNewIconNode(1)); + GenTree* setValue = gtNewStoreLclFldNode(resultTmp, valueType, valueOffset, boxedContent); + GenTree* unboxTree = gtNewOperNode(GT_COMMA, TYP_VOID, setHasValue, setValue); + + // Fallback helper call + // TODO: Mark as no-return when appropriate + GenTreeLclFld* resultAddr = gtNewLclAddrNode(resultTmp, 0); + GenTreeCall* helperCall = gtNewHelperCallNode(CORINFO_HELP_UNBOX_NULLABLE, TYP_VOID, resultAddr, + gtNewIconEmbClsHndNode(nullableCls), gtCloneExpr(objClone)); + + // Nested QMARK - "obj->pMT == ? unboxTree : helperCall" + GenTree* unboxTypeNode = gtNewIconEmbClsHndNode(unboxType); + GenTree* objMT = gtNewMethodTableLookup(objClone); + GenTree* mtLookupCond = gtNewOperNode(GT_EQ, TYP_INT, objMT, unboxTypeNode); + GenTreeColon* mtCheckColon = gtNewColonNode(TYP_VOID, unboxTree, helperCall); + GenTree* mtCheckQmark = gtNewQmarkNode(TYP_VOID, mtLookupCond, mtCheckColon); + + // Zero initialize the result in case of "obj == null" + GenTreeLclVar* zeroInitResultNode = gtNewStoreLclVarNode(resultTmp, gtNewIconNode(0)); + + // Root condition - "obj == null ? zeroInitResultNode : mtCheckQmark" + GenTree* nullcheck = gtNewOperNode(GT_EQ, TYP_INT, obj, gtNewNull()); + GenTreeColon* nullCheckColon = gtNewColonNode(TYP_VOID, zeroInitResultNode, mtCheckQmark); + GenTree* nullCheckQmark = gtNewQmarkNode(TYP_VOID, nullcheck, nullCheckColon); + + // Spill the root QMARK and return the result local + impAppendTree(nullCheckQmark, CHECK_SPILL_ALL, impCurStmtDI); + return gtNewLclvNode(resultTmp, TYP_STRUCT); +} + //------------------------------------------------------------------------ // impStoreNullableFields: create a Nullable object and store // 'hasValue' (always true) and the given value for 'value' field @@ -10098,6 +10187,11 @@ void Compiler::impImportBlockCode(BasicBlock* block) op2 = gtNewIconNode(TARGET_POINTER_SIZE, TYP_I_IMPL); op1 = gtNewOperNode(GT_ADD, TYP_BYREF, cloneOperand, op2); } + else if (helper == CORINFO_HELP_UNBOX_NULLABLE && !eeIsSharedInst(resolvedToken.hClass)) + { + // TODO: consider enabling this for Nullable> as well. + op1 = impUnboxNullable(resolvedToken.hClass, op1); + } else { // Don't optimize, just call the helper and be done with it From 49aca9534e1d585540cf464c6ecbe39c2b9b1360 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 18 Jul 2024 11:37:40 +0200 Subject: [PATCH 2/5] reorder blocks --- src/coreclr/jit/importer.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index cbac04e1965736..7f15daf076b027 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -2904,17 +2904,18 @@ GenTree* Compiler::impUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenTree* o // Nested QMARK - "obj->pMT == ? unboxTree : helperCall" GenTree* unboxTypeNode = gtNewIconEmbClsHndNode(unboxType); GenTree* objMT = gtNewMethodTableLookup(objClone); - GenTree* mtLookupCond = gtNewOperNode(GT_EQ, TYP_INT, objMT, unboxTypeNode); - GenTreeColon* mtCheckColon = gtNewColonNode(TYP_VOID, unboxTree, helperCall); - GenTree* mtCheckQmark = gtNewQmarkNode(TYP_VOID, mtLookupCond, mtCheckColon); + GenTree* mtLookupCond = gtNewOperNode(GT_NE, TYP_INT, objMT, unboxTypeNode); + GenTreeColon* mtCheckColon = gtNewColonNode(TYP_VOID, helperCall, unboxTree); + GenTreeQmark* mtCheckQmark = gtNewQmarkNode(TYP_VOID, mtLookupCond, mtCheckColon); + mtCheckQmark->SetThenNodeLikelihood(0); // Zero initialize the result in case of "obj == null" GenTreeLclVar* zeroInitResultNode = gtNewStoreLclVarNode(resultTmp, gtNewIconNode(0)); // Root condition - "obj == null ? zeroInitResultNode : mtCheckQmark" - GenTree* nullcheck = gtNewOperNode(GT_EQ, TYP_INT, obj, gtNewNull()); - GenTreeColon* nullCheckColon = gtNewColonNode(TYP_VOID, zeroInitResultNode, mtCheckQmark); - GenTree* nullCheckQmark = gtNewQmarkNode(TYP_VOID, nullcheck, nullCheckColon); + GenTree* nullcheck = gtNewOperNode(GT_NE, TYP_INT, obj, gtNewNull()); + GenTreeColon* nullCheckColon = gtNewColonNode(TYP_VOID, mtCheckQmark, zeroInitResultNode); + GenTreeQmark* nullCheckQmark = gtNewQmarkNode(TYP_VOID, nullcheck, nullCheckColon); // Spill the root QMARK and return the result local impAppendTree(nullCheckQmark, CHECK_SPILL_ALL, impCurStmtDI); From 9ef585b48dd7c709b4a3275ca7b5ffbfa18fc492 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 18 Jul 2024 12:25:51 +0200 Subject: [PATCH 3/5] Only with optimizations and in hot blocks --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/importer.cpp | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f8b500dac2c235..7ce2d86d1896f7 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4514,7 +4514,7 @@ class Compiler GenTree* impStoreNullableFields(CORINFO_CLASS_HANDLE nullableCls, GenTree* value); - GenTree* impUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenTree* obj); + GenTree* impInlineUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenTree* obj); void impLoadNullableFields(GenTree* nullableObj, CORINFO_CLASS_HANDLE nullableCls, GenTree** hasValueFld, GenTree** valueFld); int impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 7f15daf076b027..6a8628f4260233 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -2833,7 +2833,7 @@ GenTree* Compiler::impImportLdvirtftn(GenTree* thisPtr, } //------------------------------------------------------------------------ -// impUnboxNullable: Generate code for unboxing Nullable from an object (obj) +// impInlineUnboxNullable: Generate code for unboxing Nullable from an object (obj) // We're going to emit the following IR: // // Nullable result; @@ -2858,7 +2858,7 @@ GenTree* Compiler::impImportLdvirtftn(GenTree* thisPtr, // Return Value: // A local node representing the unboxed value (Nullable) // -GenTree* Compiler::impUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenTree* obj) +GenTree* Compiler::impInlineUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenTree* obj) { assert(info.compCompHnd->isNullableType(nullableCls) == TypeCompareState::Must); @@ -10188,10 +10188,11 @@ void Compiler::impImportBlockCode(BasicBlock* block) op2 = gtNewIconNode(TARGET_POINTER_SIZE, TYP_I_IMPL); op1 = gtNewOperNode(GT_ADD, TYP_BYREF, cloneOperand, op2); } - else if (helper == CORINFO_HELP_UNBOX_NULLABLE && !eeIsSharedInst(resolvedToken.hClass)) + else if (shouldExpandInline && (helper == CORINFO_HELP_UNBOX_NULLABLE) && + !eeIsSharedInst(resolvedToken.hClass)) { // TODO: consider enabling this for Nullable> as well. - op1 = impUnboxNullable(resolvedToken.hClass, op1); + op1 = impInlineUnboxNullable(resolvedToken.hClass, op1); } else { From 2918411ddc60214b009b18c07cf55f7ed21f1c14 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 19 Jul 2024 00:55:46 +0200 Subject: [PATCH 4/5] clean up, address feedback --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/importer.cpp | 73 ++++++++++++++++++------------------ 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 7ce2d86d1896f7..6115bd18795d68 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4514,7 +4514,7 @@ class Compiler GenTree* impStoreNullableFields(CORINFO_CLASS_HANDLE nullableCls, GenTree* value); - GenTree* impInlineUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenTree* obj); + GenTree* impInlineUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenTree* nullableClsNode, GenTree* obj); void impLoadNullableFields(GenTree* nullableObj, CORINFO_CLASS_HANDLE nullableCls, GenTree** hasValueFld, GenTree** valueFld); int impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 6a8628f4260233..3908f16edeb8c4 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -2834,7 +2834,8 @@ GenTree* Compiler::impImportLdvirtftn(GenTree* thisPtr, //------------------------------------------------------------------------ // impInlineUnboxNullable: Generate code for unboxing Nullable from an object (obj) -// We're going to emit the following IR: +// We either inline the unbox operation (if profitable) or call the helper. +// The inline expansion is as follows: // // Nullable result; // if (obj == null) @@ -2852,22 +2853,42 @@ GenTree* Compiler::impImportLdvirtftn(GenTree* thisPtr, // } // // Arguments: -// nullableCls - class handle representing the Nullable type -// obj - object to unbox +// nullableCls - class handle representing the Nullable type +// nullableClsNode - tree node representing the Nullable type (can be a runtime lookup tree) +// obj - object to unbox // // Return Value: // A local node representing the unboxed value (Nullable) // -GenTree* Compiler::impInlineUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenTree* obj) +GenTree* Compiler::impInlineUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenTree* nullableClsNode, GenTree* obj) { assert(info.compCompHnd->isNullableType(nullableCls) == TypeCompareState::Must); unsigned resultTmp = lvaGrabTemp(true DEBUGARG("Nullable tmp")); lvaSetStruct(resultTmp, nullableCls, false); lvaGetDesc(resultTmp)->lvHasLdAddrOp = true; + GenTreeLclFld* resultAddr = gtNewLclAddrNode(resultTmp, 0); - // The underlying type of the nullable: - CORINFO_CLASS_HANDLE unboxType = info.compCompHnd->getTypeForBox(nullableCls); + // Check profitability of inlining the unbox operation + bool shouldExpandInline = compCurBB->isRunRarely() && opts.OptimizationEnabled() && !eeIsSharedInst(nullableCls); + + // It's less profitable to inline the unbox operation if the underlying type is too large + CORINFO_CLASS_HANDLE unboxType = NO_CLASS_HANDLE; + if (shouldExpandInline) + { + // The underlying type of the nullable: + unboxType = info.compCompHnd->getTypeForBox(nullableCls); + shouldExpandInline = info.compCompHnd->getClassSize(unboxType) <= getUnrollThreshold(Memcpy); + } + + if (!shouldExpandInline) + { + // No expansion needed, just call the helper + GenTreeCall* call = + gtNewHelperCallNode(CORINFO_HELP_UNBOX_NULLABLE, TYP_VOID, resultAddr, nullableClsNode, obj); + impAppendTree(call, CHECK_SPILL_ALL, impCurStmtDI); + return gtNewLclvNode(resultTmp, TYP_STRUCT); + } // Clone the object (and spill side effects) GenTree* objClone; @@ -2897,11 +2918,11 @@ GenTree* Compiler::impInlineUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenT // Fallback helper call // TODO: Mark as no-return when appropriate - GenTreeLclFld* resultAddr = gtNewLclAddrNode(resultTmp, 0); - GenTreeCall* helperCall = gtNewHelperCallNode(CORINFO_HELP_UNBOX_NULLABLE, TYP_VOID, resultAddr, - gtNewIconEmbClsHndNode(nullableCls), gtCloneExpr(objClone)); + GenTreeCall* helperCall = + gtNewHelperCallNode(CORINFO_HELP_UNBOX_NULLABLE, TYP_VOID, resultAddr, nullableClsNode, gtCloneExpr(objClone)); // Nested QMARK - "obj->pMT == ? unboxTree : helperCall" + assert(unboxType != NO_CLASS_HANDLE); GenTree* unboxTypeNode = gtNewIconEmbClsHndNode(unboxType); GenTree* objMT = gtNewMethodTableLookup(objClone); GenTree* mtLookupCond = gtNewOperNode(GT_NE, TYP_INT, objMT, unboxTypeNode); @@ -10188,11 +10209,11 @@ void Compiler::impImportBlockCode(BasicBlock* block) op2 = gtNewIconNode(TARGET_POINTER_SIZE, TYP_I_IMPL); op1 = gtNewOperNode(GT_ADD, TYP_BYREF, cloneOperand, op2); } - else if (shouldExpandInline && (helper == CORINFO_HELP_UNBOX_NULLABLE) && - !eeIsSharedInst(resolvedToken.hClass)) + else if (helper == CORINFO_HELP_UNBOX_NULLABLE) { - // TODO: consider enabling this for Nullable> as well. - op1 = impInlineUnboxNullable(resolvedToken.hClass, op1); + // op1 is the object being unboxed + // op2 is either a class handle node or a runtime lookup node (it's fine to reorder) + op1 = impInlineUnboxNullable(resolvedToken.hClass, op2, op1); } else { @@ -10200,30 +10221,8 @@ void Compiler::impImportBlockCode(BasicBlock* block) JITDUMP("\n Importing %s as helper call because %s\n", opcode == CEE_UNBOX ? "UNBOX" : "UNBOX.ANY", canExpandInline ? "want smaller code or faster jitting" : "inline expansion not legal"); - if (helper == CORINFO_HELP_UNBOX) - { - op1 = gtNewHelperCallNode(helper, TYP_BYREF, op2, op1); - } - else - { - assert(helper == CORINFO_HELP_UNBOX_NULLABLE); - - // We're going to emit the following sequence of IR: - // - // Nullable result; - // void CORINFO_HELP_UNBOX_NULLABLE(&result, unboxCls, obj); - // push result; - // - unsigned resultTmp = lvaGrabTemp(true DEBUGARG("Nullable tmp")); - lvaSetStruct(resultTmp, resolvedToken.hClass, false); - lvaGetDesc(resultTmp)->lvHasLdAddrOp = true; - - GenTreeLclFld* resultAddr = gtNewLclAddrNode(resultTmp, 0); - // NOTE: it's fine for op2 to be evaluated before op1 - GenTreeCall* helperCall = gtNewHelperCallNode(helper, TYP_VOID, resultAddr, op2, op1); - impAppendTree(helperCall, CHECK_SPILL_ALL, impCurStmtDI); - op1 = gtNewLclvNode(resultTmp, TYP_STRUCT); - } + assert(helper == CORINFO_HELP_UNBOX); + op1 = gtNewHelperCallNode(helper, TYP_BYREF, op2, op1); } assert((helper == CORINFO_HELP_UNBOX && op1->gtType == TYP_BYREF) || // Unbox helper returns a byref. From d5c1948c448a6b0ee7161ec4cf635e64ade07f68 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 19 Jul 2024 11:00:55 +0200 Subject: [PATCH 5/5] Update importer.cpp --- src/coreclr/jit/importer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 3908f16edeb8c4..ad0c4d2b902813 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -2870,7 +2870,7 @@ GenTree* Compiler::impInlineUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenT GenTreeLclFld* resultAddr = gtNewLclAddrNode(resultTmp, 0); // Check profitability of inlining the unbox operation - bool shouldExpandInline = compCurBB->isRunRarely() && opts.OptimizationEnabled() && !eeIsSharedInst(nullableCls); + bool shouldExpandInline = !compCurBB->isRunRarely() && opts.OptimizationEnabled() && !eeIsSharedInst(nullableCls); // It's less profitable to inline the unbox operation if the underlying type is too large CORINFO_CLASS_HANDLE unboxType = NO_CLASS_HANDLE;