diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 36acfe6adecde0..97bf11f681047a 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4020,8 +4020,6 @@ class Compiler CORINFO_CALL_INFO* callInfo, IL_OFFSET rawILOffset); - CORINFO_CLASS_HANDLE impGetSpecialIntrinsicExactReturnType(CORINFO_METHOD_HANDLE specialIntrinsicHandle); - bool impMethodInfo_hasRetBuffArg(CORINFO_METHOD_INFO* methInfo, CorInfoCallConvExtension callConv); GenTree* impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HANDLE retClsHnd); @@ -4069,7 +4067,6 @@ class Compiler var_types callType, NamedIntrinsic intrinsicName, bool tailCall); - NamedIntrinsic lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method); GenTree* impUnsupportedNamedIntrinsic(unsigned helper, CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO* sig, @@ -4170,6 +4167,7 @@ class Compiler CHECK_SPILL_NONE = -2 }; + NamedIntrinsic lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method); void impBeginTreeList(); void impEndTreeList(BasicBlock* block, Statement* firstStmt, Statement* lastStmt); void impEndTreeList(BasicBlock* block); diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 29aabbaf40a902..d663cb1c6840a7 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -1623,8 +1623,7 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) if (argInfo.argHasSideEff) { noway_assert(argInfo.argIsUsed == false); - newStmt = nullptr; - bool append = true; + newStmt = nullptr; if (argNode->gtOper == GT_OBJ || argNode->gtOper == GT_MKREFANY) { @@ -1633,92 +1632,22 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) // Just hang the address here in case there are side-effect. newStmt = gtNewStmt(gtUnusedValNode(argNode->AsOp()->gtOp1), callILOffset); } - else + + // If we don't have something custom to append, + // just append the arg node as an unused value. + if (newStmt == nullptr) { - // In some special cases, unused args with side effects can - // trigger further changes. - // - // (1) If the arg is a static field access and the field access - // was produced by a call to EqualityComparer.get_Default, the - // helper call to ensure the field has a value can be suppressed. - // This helper call is marked as a "Special DCE" helper during - // importation, over in fgGetStaticsCCtorHelper. - // - // (2) NYI. If, after tunneling through GT_RET_VALs, we find that - // the actual arg expression has no side effects, we can skip - // appending all together. This will help jit TP a bit. - // - // Chase through any GT_RET_EXPRs to find the actual argument - // expression. - GenTree* actualArgNode = argNode->gtRetExprVal(&bbFlags); - - // For case (1) - // - // Look for the following tree shapes - // prejit: (IND (ADD (CONST, CALL(special dce helper...)))) - // jit : (COMMA (CALL(special dce helper...), (FIELD ...))) - if (actualArgNode->gtOper == GT_COMMA) - { - // Look for (COMMA (CALL(special dce helper...), (FIELD ...))) - GenTree* op1 = actualArgNode->AsOp()->gtOp1; - GenTree* op2 = actualArgNode->AsOp()->gtOp2; - if (op1->IsCall() && - ((op1->AsCall()->gtCallMoreFlags & GTF_CALL_M_HELPER_SPECIAL_DCE) != 0) && - (op2->gtOper == GT_FIELD) && ((op2->gtFlags & GTF_EXCEPT) == 0)) - { - JITDUMP("\nPerforming special dce on unused arg [%06u]:" - " actual arg [%06u] helper call [%06u]\n", - argNode->gtTreeID, actualArgNode->gtTreeID, op1->gtTreeID); - // Drop the whole tree - append = false; - } - } - else if (actualArgNode->gtOper == GT_IND) - { - // Look for (IND (ADD (CONST, CALL(special dce helper...)))) - GenTree* addr = actualArgNode->AsOp()->gtOp1; - - if (addr->gtOper == GT_ADD) - { - GenTree* op1 = addr->AsOp()->gtOp1; - GenTree* op2 = addr->AsOp()->gtOp2; - if (op1->IsCall() && - ((op1->AsCall()->gtCallMoreFlags & GTF_CALL_M_HELPER_SPECIAL_DCE) != 0) && - op2->IsCnsIntOrI()) - { - // Drop the whole tree - JITDUMP("\nPerforming special dce on unused arg [%06u]:" - " actual arg [%06u] helper call [%06u]\n", - argNode->gtTreeID, actualArgNode->gtTreeID, op1->gtTreeID); - append = false; - } - } - } + newStmt = gtNewStmt(gtUnusedValNode(argNode), callILOffset); } - if (!append) + fgInsertStmtAfter(block, afterStmt, newStmt); + afterStmt = newStmt; +#ifdef DEBUG + if (verbose) { - assert(newStmt == nullptr); - JITDUMP("Arg tree side effects were discardable, not appending anything for arg\n"); + gtDispStmt(afterStmt); } - else - { - // If we don't have something custom to append, - // just append the arg node as an unused value. - if (newStmt == nullptr) - { - newStmt = gtNewStmt(gtUnusedValNode(argNode), callILOffset); - } - - fgInsertStmtAfter(block, afterStmt, newStmt); - afterStmt = newStmt; -#ifdef DEBUG - if (verbose) - { - gtDispStmt(afterStmt); - } #endif // DEBUG - } } else if (argNode->IsBoxedValue()) { diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 04faed33f50a87..1f3e71ade8ab01 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -910,21 +910,6 @@ GenTreeCall* Compiler::fgGetStaticsCCtorHelper(CORINFO_CLASS_HANDLE cls, CorInfo GenTreeCall* result = gtNewHelperCallNode(helper, type, argList); result->gtFlags |= callFlags; - - // If we're importing the special EqualityComparer.Default or Comparer.Default - // intrinsics, flag the helper call. Later during inlining, we can - // remove the helper call if the associated field lookup is unused. - if ((info.compFlags & CORINFO_FLG_JIT_INTRINSIC) != 0) - { - NamedIntrinsic ni = lookupNamedIntrinsic(info.compMethodHnd); - if ((ni == NI_System_Collections_Generic_EqualityComparer_get_Default) || - (ni == NI_System_Collections_Generic_Comparer_get_Default)) - { - JITDUMP("\nmarking helper call [%06u] as special dce...\n", result->gtTreeID); - result->gtCallMoreFlags |= GTF_CALL_M_HELPER_SPECIAL_DCE; - } - } - return result; } diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 35b01ad27ec439..b56372d18d28fc 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -882,6 +882,17 @@ bool GenTreeCall::IsPure(Compiler* compiler) const // true if this call has any side-effects; false otherwise. bool GenTreeCall::HasSideEffects(Compiler* compiler, bool ignoreExceptions, bool ignoreCctors) const { + // Some named intrinsics are known to have ignorable side effects. + if (gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) + { + NamedIntrinsic ni = compiler->lookupNamedIntrinsic(gtCallMethHnd); + if ((ni == NI_System_Collections_Generic_Comparer_get_Default) || + (ni == NI_System_Collections_Generic_EqualityComparer_get_Default)) + { + return false; + } + } + // Generally all GT_CALL nodes are considered to have side-effects, but we may have extra information about helper // calls that can prove them side-effect-free. if (gtCallType != CT_HELPER) @@ -15920,9 +15931,10 @@ void Compiler::gtExtractSideEffList(GenTree* expr, } // Generally all GT_CALL nodes are considered to have side-effects. - // So if we get here it must be a helper call that we decided it does + // So if we get here it must be a helper call or a special intrinsic that we decided it does // not have side effects that we needed to keep. - assert(!node->OperIs(GT_CALL) || (node->AsCall()->gtCallType == CT_HELPER)); + assert(!node->OperIs(GT_CALL) || (node->AsCall()->gtCallType == CT_HELPER) || + (node->AsCall()->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)); } if ((m_flags & GTF_IS_IN_CSE) != 0) @@ -17743,13 +17755,50 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b break; } - CORINFO_CLASS_HANDLE specialObjClass = impGetSpecialIntrinsicExactReturnType(call->gtCallMethHnd); - if (specialObjClass != nullptr) + // Try to get the actual type of [Equality]Comparer<>.Default + if ((ni == NI_System_Collections_Generic_Comparer_get_Default) || + (ni == NI_System_Collections_Generic_EqualityComparer_get_Default)) { - objClass = specialObjClass; - *pIsExact = true; - *pIsNonNull = true; - break; + CORINFO_SIG_INFO sig; + info.compCompHnd->getMethodSig(call->gtCallMethHnd, &sig); + assert(sig.sigInst.classInstCount == 1); + CORINFO_CLASS_HANDLE typeHnd = sig.sigInst.classInst[0]; + assert(typeHnd != nullptr); + + // Lookup can incorrect when we have __Canon as it won't appear to implement any interface types. + // And if we do not have a final type, devirt & inlining is unlikely to result in much + // simplification. We can use CORINFO_FLG_FINAL to screen out both of these cases. + const DWORD typeAttribs = info.compCompHnd->getClassAttribs(typeHnd); + const bool isFinalType = ((typeAttribs & CORINFO_FLG_FINAL) != 0); + + if (isFinalType) + { + if (ni == NI_System_Collections_Generic_EqualityComparer_get_Default) + { + objClass = info.compCompHnd->getDefaultEqualityComparerClass(typeHnd); + } + else + { + assert(ni == NI_System_Collections_Generic_Comparer_get_Default); + objClass = info.compCompHnd->getDefaultComparerClass(typeHnd); + } + } + + if (objClass == nullptr) + { + // Don't re-visit this intrinsic in this case. + call->gtCallMoreFlags &= ~GTF_CALL_M_SPECIAL_INTRINSIC; + JITDUMP("Special intrinsic for type %s: type not final, so deferring opt\n", + eeGetClassName(typeHnd)) + } + else + { + JITDUMP("Special intrinsic for type %s: return type is %s\n", eeGetClassName(typeHnd), + eeGetClassName(objClass)) + *pIsExact = true; + *pIsNonNull = true; + break; + } } } if (call->IsInlineCandidate()) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index be5d7769cbb0aa..b043956468848c 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -3796,8 +3796,6 @@ enum GenTreeCallFlags : unsigned int // to restore real function address and load hidden argument // as the first argument for calli. It is CoreRT replacement for instantiating // stubs, because executable code cannot be generated at runtime. - GTF_CALL_M_HELPER_SPECIAL_DCE = 0x00020000, // this helper call can be removed if it is part of a comma and - // the comma result is unused. GTF_CALL_M_DEVIRTUALIZED = 0x00040000, // this call was devirtualized GTF_CALL_M_UNBOXED = 0x00080000, // this call was optimized to use the unboxed entry point GTF_CALL_M_GUARDED_DEVIRT = 0x00100000, // this call is a candidate for guarded devirtualization diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index e2a4257897d277..d49a12ffd13dd0 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -19254,6 +19254,19 @@ void Compiler::impCheckCanInline(GenTreeCall* call, goto _exit; } + // It's better for JIT to keep these methods not inlined for CQ. + NamedIntrinsic ni; + if (pParam->call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) + { + ni = pParam->pThis->lookupNamedIntrinsic(pParam->call->gtCallMethHnd); + if ((ni == NI_System_Collections_Generic_Comparer_get_Default) || + (ni == NI_System_Collections_Generic_EqualityComparer_get_Default)) + { + pParam->result->NoteFatal(InlineObservation::CALLEE_SPECIAL_INTRINSIC); + goto _exit; + } + } + // Speculatively check if initClass() can be done. // If it can be done, we will try to inline the method. initClassResult = @@ -21493,79 +21506,6 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, #endif // FEATURE_READYTORUN_COMPILER } -//------------------------------------------------------------------------ -// impGetSpecialIntrinsicExactReturnType: Look for special cases where a call -// to an intrinsic returns an exact type -// -// Arguments: -// methodHnd -- handle for the special intrinsic method -// -// Returns: -// Exact class handle returned by the intrinsic call, if known. -// Nullptr if not known, or not likely to lead to beneficial optimization. - -CORINFO_CLASS_HANDLE Compiler::impGetSpecialIntrinsicExactReturnType(CORINFO_METHOD_HANDLE methodHnd) -{ - JITDUMP("Special intrinsic: looking for exact type returned by %s\n", eeGetMethodFullName(methodHnd)); - - CORINFO_CLASS_HANDLE result = nullptr; - - // See what intrinisc we have... - const NamedIntrinsic ni = lookupNamedIntrinsic(methodHnd); - switch (ni) - { - case NI_System_Collections_Generic_Comparer_get_Default: - case NI_System_Collections_Generic_EqualityComparer_get_Default: - { - // Expect one class generic parameter; figure out which it is. - CORINFO_SIG_INFO sig; - info.compCompHnd->getMethodSig(methodHnd, &sig); - assert(sig.sigInst.classInstCount == 1); - CORINFO_CLASS_HANDLE typeHnd = sig.sigInst.classInst[0]; - assert(typeHnd != nullptr); - - // Lookup can incorrect when we have __Canon as it won't appear - // to implement any interface types. - // - // And if we do not have a final type, devirt & inlining is - // unlikely to result in much simplification. - // - // We can use CORINFO_FLG_FINAL to screen out both of these cases. - const DWORD typeAttribs = info.compCompHnd->getClassAttribs(typeHnd); - const bool isFinalType = ((typeAttribs & CORINFO_FLG_FINAL) != 0); - - if (isFinalType) - { - if (ni == NI_System_Collections_Generic_EqualityComparer_get_Default) - { - result = info.compCompHnd->getDefaultEqualityComparerClass(typeHnd); - } - else - { - assert(ni == NI_System_Collections_Generic_Comparer_get_Default); - result = info.compCompHnd->getDefaultComparerClass(typeHnd); - } - JITDUMP("Special intrinsic for type %s: return type is %s\n", eeGetClassName(typeHnd), - result != nullptr ? eeGetClassName(result) : "unknown"); - } - else - { - JITDUMP("Special intrinsic for type %s: type not final, so deferring opt\n", eeGetClassName(typeHnd)); - } - - break; - } - - default: - { - JITDUMP("This special intrinsic not handled, sorry...\n"); - break; - } - } - - return result; -} - //------------------------------------------------------------------------ // impAllocateToken: create CORINFO_RESOLVED_TOKEN into jit-allocated memory and init it. // diff --git a/src/coreclr/jit/inline.def b/src/coreclr/jit/inline.def index f21e4f929ea95e..a16d82888b64a3 100644 --- a/src/coreclr/jit/inline.def +++ b/src/coreclr/jit/inline.def @@ -57,6 +57,7 @@ INLINE_OBSERVATION(STFLD_NEEDS_HELPER, bool, "stfld needs helper", INLINE_OBSERVATION(TOO_MANY_ARGUMENTS, bool, "too many arguments", FATAL, CALLEE) INLINE_OBSERVATION(TOO_MANY_LOCALS, bool, "too many locals", FATAL, CALLEE) INLINE_OBSERVATION(EXPLICIT_TAIL_PREFIX, bool, "explicit tail prefix in callee", FATAL, CALLEE) +INLINE_OBSERVATION(SPECIAL_INTRINSIC, bool, "skipped for special intrinsic", FATAL, CALLEE) // ------ Callee Performance ------- diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 110730f15e6c5a..c69e8bc3cbd1db 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -9629,6 +9629,34 @@ void Compiler::fgValueNumberCall(GenTreeCall* call) } else { + if (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) + { + NamedIntrinsic ni = lookupNamedIntrinsic(call->gtCallMethHnd); + if ((ni == NI_System_Collections_Generic_Comparer_get_Default) || + (ni == NI_System_Collections_Generic_EqualityComparer_get_Default)) + { + bool isExact = false; + bool isNotNull = false; + CORINFO_CLASS_HANDLE cls = gtGetClassHandle(call, &isExact, &isNotNull); + if ((cls != nullptr) && isExact && isNotNull) + { + ValueNum clsVN = vnStore->VNForHandle(ssize_t(cls), GTF_ICON_CLASS_HDL); + ValueNum funcVN; + if (ni == NI_System_Collections_Generic_EqualityComparer_get_Default) + { + funcVN = vnStore->VNForFunc(call->TypeGet(), VNF_GetDefaultEqualityComparer, clsVN); + } + else + { + assert(ni == NI_System_Collections_Generic_Comparer_get_Default); + funcVN = vnStore->VNForFunc(call->TypeGet(), VNF_GetDefaultComparer, clsVN); + } + call->gtVNPair.SetBoth(funcVN); + return; + } + } + } + if (call->TypeGet() == TYP_VOID) { call->gtVNPair.SetBoth(ValueNumStore::VNForVoid()); diff --git a/src/coreclr/jit/valuenumfuncs.h b/src/coreclr/jit/valuenumfuncs.h index aa13273c8773f9..2ddfff34dfa455 100644 --- a/src/coreclr/jit/valuenumfuncs.h +++ b/src/coreclr/jit/valuenumfuncs.h @@ -128,6 +128,9 @@ ValueNumFuncDef(GetsharedNongcthreadstaticBaseNoctor, 2, false, true, true) ValueNumFuncDef(GetsharedGcthreadstaticBaseDynamicclass, 2, false, true, true) ValueNumFuncDef(GetsharedNongcthreadstaticBaseDynamicclass, 2, false, true, true) +ValueNumFuncDef(GetDefaultComparer, 1, false, true, false) +ValueNumFuncDef(GetDefaultEqualityComparer, 1, false, true, false) + ValueNumFuncDef(ClassinitSharedDynamicclass, 2, false, false, false) ValueNumFuncDef(RuntimeHandleMethod, 2, false, true, false) ValueNumFuncDef(RuntimeHandleClass, 2, false, true, false)