JIT: keep ref count for runtime generic context lookups
authorAndy Ayers <andya@microsoft.com>
Sat, 18 Feb 2017 04:51:59 +0000 (20:51 -0800)
committerAndy Ayers <andya@microsoft.com>
Fri, 24 Feb 2017 00:23:18 +0000 (16:23 -0800)
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
src/coreclr/src/jit/compiler.h
src/coreclr/src/jit/compiler.hpp
src/coreclr/src/jit/flowgraph.cpp
src/coreclr/src/jit/importer.cpp
src/coreclr/src/jit/inline.h
src/coreclr/src/jit/lclvars.cpp
src/coreclr/src/jit/morph.cpp

index a9eba55..5d88fdb 100644 (file)
@@ -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 */
index 5b8bca4..cc1caec 100644 (file)
@@ -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,
index beb3c79..3a6dd82 100644 (file)
@@ -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;
         }
index 392ada1..ffec180 100644 (file)
@@ -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())
     {
index 743cae2..24ba90a 100644 (file)
@@ -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.
index 7bd0ae4..bf4d804 100644 (file)
@@ -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;
index f4cd13c..3f9ed21 100644 (file)
@@ -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
 
index f41f6ee..0035b9d 100644 (file)
@@ -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)
         {