From f8f8947d25212d679fa85a661d92cceabd665de3 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 17 Feb 2017 20:51:59 -0800 Subject: [PATCH] JIT: keep ref count for runtime generic context lookups Keep a ref count for runtime generic context lookups, and track when an inlinee requires a runtime lookup but then ignores the resulting context. Only report/keep the context alive if there are known uses. This pattern happens frequently in inlined generic code that uses type tests to extract properties of types. Addresses dotnet/coreclr#1691. Commit migrated from https://github.com/dotnet/coreclr/commit/5a1e02cd6f056cebdd1c240d400ac7a34234d43a --- src/coreclr/src/jit/compiler.cpp | 2 -- src/coreclr/src/jit/compiler.h | 7 +++++-- src/coreclr/src/jit/compiler.hpp | 28 +++++++++++++++++++++------- src/coreclr/src/jit/flowgraph.cpp | 24 +++++++++++++++++++++++- src/coreclr/src/jit/importer.cpp | 14 ++++++++++---- src/coreclr/src/jit/inline.h | 4 +++- src/coreclr/src/jit/lclvars.cpp | 2 ++ src/coreclr/src/jit/morph.cpp | 2 +- 8 files changed, 65 insertions(+), 18 deletions(-) diff --git a/src/coreclr/src/jit/compiler.cpp b/src/coreclr/src/jit/compiler.cpp index a9eba55..5d88fdb 100644 --- a/src/coreclr/src/jit/compiler.cpp +++ b/src/coreclr/src/jit/compiler.cpp @@ -5663,8 +5663,6 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE classPtr, info.compCallUnmanaged = 0; info.compLvFrameListRoot = BAD_VAR_NUM; - lvaGenericsContextUsed = false; - info.compInitMem = ((methodInfo->options & CORINFO_OPT_INIT_LOCALS) != 0); /* Allocate the local variable table */ diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index 5b8bca4..cc1caec 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -2435,7 +2435,7 @@ public: unsigned lvaCallEspCheck; // confirms ESP not corrupted after a call #endif - bool lvaGenericsContextUsed; + unsigned lvaGenericsContextUseCount; bool lvaKeepAliveAndReportThis(); // Synchronized instance method of a reference type, or // CORINFO_GENERICS_CTXT_FROM_THIS? @@ -3322,7 +3322,10 @@ private: GenTreePtr variableBeingDereferenced, InlArgInfo* inlArgInfo); - void impMarkInlineCandidate(GenTreePtr call, CORINFO_CONTEXT_HANDLE exactContextHnd, CORINFO_CALL_INFO* callInfo); + void impMarkInlineCandidate(GenTreePtr call, + CORINFO_CONTEXT_HANDLE exactContextHnd, + bool exactContextNeedsRuntimeLookup, + CORINFO_CALL_INFO* callInfo); bool impTailCallRetTypeCompatible(var_types callerRetType, CORINFO_CLASS_HANDLE callerRetTypeClass, diff --git a/src/coreclr/src/jit/compiler.hpp b/src/coreclr/src/jit/compiler.hpp index beb3c79..3a6dd82 100644 --- a/src/coreclr/src/jit/compiler.hpp +++ b/src/coreclr/src/jit/compiler.hpp @@ -2199,11 +2199,14 @@ inline bool Compiler::lvaKeepAliveAndReportThis() return false; } + const bool genericsContextIsThis = (info.compMethodInfo->options & CORINFO_GENERICS_CTXT_FROM_THIS) != 0; + #ifdef JIT32_GCENCODER + if (info.compFlags & CORINFO_FLG_SYNCH) return true; - if (info.compMethodInfo->options & CORINFO_GENERICS_CTXT_FROM_THIS) + if (genericsContextIsThis) { // TODO: Check if any of the exception clauses are // typed using a generic type. Else, we do not need to report this. @@ -2213,18 +2216,29 @@ inline bool Compiler::lvaKeepAliveAndReportThis() if (opts.compDbgCode) return true; - if (lvaGenericsContextUsed) + if (lvaGenericsContextUseCount > 0) + { + JITDUMP("Reporting this as generic context: %u refs\n", lvaGenericsContextUseCount); return true; + } } #else // !JIT32_GCENCODER // If the generics context is the this pointer we need to report it if either // the VM requires us to keep the generics context alive or it is used in a look-up. - // We keep it alive in the lookup scenario, even when the VM didn't ask us too + // We keep it alive in the lookup scenario, even when the VM didn't ask us to, // because collectible types need the generics context when gc-ing. - if ((info.compMethodInfo->options & CORINFO_GENERICS_CTXT_FROM_THIS) && - (lvaGenericsContextUsed || (info.compMethodInfo->options & CORINFO_GENERICS_CTXT_KEEP_ALIVE))) + if (genericsContextIsThis) { - return true; + const bool isUsed = lvaGenericsContextUseCount > 0; + const bool mustKeep = (info.compMethodInfo->options & CORINFO_GENERICS_CTXT_KEEP_ALIVE) != 0; + + if (isUsed || mustKeep) + { + JITDUMP("Reporting this as generic context: %u refs%s\n", lvaGenericsContextUseCount, + mustKeep ? ", must keep" : ""); + + return true; + } } #endif @@ -2250,7 +2264,7 @@ inline bool Compiler::lvaReportParamTypeArg() // Otherwise, if an exact type parameter is needed in the body, report the generics context. // We do this because collectible types needs the generics context when gc-ing. - if (lvaGenericsContextUsed) + if (lvaGenericsContextUseCount > 0) { return true; } diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp index 392ada1..ffec180 100644 --- a/src/coreclr/src/jit/flowgraph.cpp +++ b/src/coreclr/src/jit/flowgraph.cpp @@ -7479,7 +7479,7 @@ GenTreePtr Compiler::fgGetCritSectOfStaticMethod() // Collectible types requires that for shared generic code, if we use the generic context paramter // that we report it. (This is a conservative approach, we could detect some cases particularly when the // context parameter is this that we don't need the eager reporting logic.) - lvaGenericsContextUsed = true; + lvaGenericsContextUseCount++; switch (kind.runtimeLookupKind) { @@ -22445,6 +22445,28 @@ GenTreePtr Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) void Compiler::fgInlineAppendStatements(InlineInfo* inlineInfo, BasicBlock* block, GenTreePtr stmtAfter) { + // If this inlinee was passed a generic context but didn't look at + // it, we can decrement the "generic context was used" ref count. + if ((inlineInfo->inlineCandidateInfo->methInfo.args.callConv & CORINFO_CALLCONV_PARAMTYPE) != 0) + { + // Did the context require the caller to perform a runtime lookup? + if (inlineInfo->inlineCandidateInfo->exactContextNeedsRuntimeLookup) + { + // Fetch the temp for the generic context + const unsigned typeCtxtArg = inlineInfo->typeContextArg; + const unsigned tmpNum = inlineInfo->lclTmpNum[typeCtxtArg]; + + // Was it used in the inline body? + if (tmpNum == BAD_VAR_NUM) + { + // No, so the runtime lookup is not needed. + JITDUMP("Inlinee ignores runtime lookup generics context\n"); + assert(lvaGenericsContextUseCount > 0); + lvaGenericsContextUseCount--; + } + } + } + // Null out any gc ref locals if (!inlineInfo->HasGcRefLocals()) { diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 743cae2..24ba90a 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -1929,7 +1929,7 @@ GenTreePtr Compiler::getRuntimeContextTree(CORINFO_RUNTIME_LOOKUP_KIND kind) // Collectible types requires that for shared generic code, if we use the generic context parameter // that we report it. (This is a conservative approach, we could detect some cases particularly when the // context parameter is this that we don't need the eager reporting logic.) - lvaGenericsContextUsed = true; + lvaGenericsContextUseCount++; if (kind == CORINFO_LOOKUP_THISOBJ) { @@ -7401,7 +7401,7 @@ var_types Compiler::impImportCall(OPCODE opcode, #endif // defined(DEBUG) || defined(INLINE_DATA) // Is it an inline candidate? - impMarkInlineCandidate(call, exactContextHnd, callInfo); + impMarkInlineCandidate(call, exactContextHnd, exactContextNeedsRuntimeLookup == TRUE, callInfo); } // append the call node. @@ -7625,7 +7625,7 @@ DONE: #endif // defined(DEBUG) || defined(INLINE_DATA) // Is it an inline candidate? - impMarkInlineCandidate(call, exactContextHnd, callInfo); + impMarkInlineCandidate(call, exactContextHnd, exactContextNeedsRuntimeLookup == TRUE, callInfo); } DONE_CALL: @@ -17461,7 +17461,8 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) // Ignore the type context argument if (hasTypeCtxtArg && (argCnt == typeCtxtArg)) { - typeCtxtArg = 0xFFFFFFFF; + pInlineInfo->typeContextArg = typeCtxtArg; + typeCtxtArg = 0xFFFFFFFF; continue; } @@ -18083,6 +18084,7 @@ BOOL Compiler::impInlineIsGuaranteedThisDerefBeforeAnySideEffects(GenTreePtr ad void Compiler::impMarkInlineCandidate(GenTreePtr callNode, CORINFO_CONTEXT_HANDLE exactContextHnd, + bool exactContextNeedsRuntimeLookup, CORINFO_CALL_INFO* callInfo) { // Let the strategy know there's another call @@ -18268,6 +18270,10 @@ void Compiler::impMarkInlineCandidate(GenTreePtr callNode, // The old value should be NULL assert(call->gtInlineCandidateInfo == nullptr); + // The new value should not be NULL. + assert(inlineCandidateInfo != nullptr); + inlineCandidateInfo->exactContextNeedsRuntimeLookup = exactContextNeedsRuntimeLookup; + call->gtInlineCandidateInfo = inlineCandidateInfo; // Mark the call node as inline candidate. diff --git a/src/coreclr/src/jit/inline.h b/src/coreclr/src/jit/inline.h index 7bd0ae4..bf4d804 100644 --- a/src/coreclr/src/jit/inline.h +++ b/src/coreclr/src/jit/inline.h @@ -506,6 +506,7 @@ struct InlineCandidateInfo var_types fncRetType; CORINFO_METHOD_HANDLE ilCallerHandle; // the logical IL caller of this inlinee. CORINFO_CONTEXT_HANDLE exactContextHnd; + bool exactContextNeedsRuntimeLookup; CorInfoInitClassResult initClassResult; }; @@ -570,7 +571,8 @@ struct InlineInfo return numberOfGcRefLocals > 0; } - bool thisDereferencedFirst; + bool thisDereferencedFirst; + unsigned typeContextArg; #ifdef FEATURE_SIMD bool hasSIMDTypeArgLocalOrReturn; diff --git a/src/coreclr/src/jit/lclvars.cpp b/src/coreclr/src/jit/lclvars.cpp index f4cd13c..3f9ed21 100644 --- a/src/coreclr/src/jit/lclvars.cpp +++ b/src/coreclr/src/jit/lclvars.cpp @@ -38,6 +38,8 @@ void Compiler::lvaInit() lvaRefCountingStarted = false; lvaLocalVarRefCounted = false; + lvaGenericsContextUseCount = 0; + lvaSortAgain = false; // false: We don't need to call lvaSortOnly() lvaTrackedFixed = false; // false: We can still add new tracked variables diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index f41f6ee2..0035b9d 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -16305,7 +16305,7 @@ GenTreePtr Compiler::fgInitThisClass() // Collectible types requires that for shared generic code, if we use the generic context paramter // that we report it. (This is a conservative approach, we could detect some cases particularly when the // context parameter is this that we don't need the eager reporting logic.) - lvaGenericsContextUsed = true; + lvaGenericsContextUseCount++; switch (kind.runtimeLookupKind) { -- 2.7.4