JIT: fix case with slow generic delegate creation (#17802)

The jit has been using IR pattern matching to find the information needed for
optimizing delegate construction. This missed one case, causing the code to
use a very slow construction path.

to convey this information to the optimization. With this change the jit now
now relies on the token cache for all the other cases too.

This initial commit preserves the pattern match code and verifies that when
it fires it reaches the same conclusion as the token cache does.

This cross-validation revealed one case where the token information was less
specific than the pattern match so we now also update the method handle in
the token from the call info.

A subsequent commit will remove the pattern matching when we are confident the
new token cache logic handles all the cases correctly.

Closes #12264.

diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp
index be6f5fd..0c7b6ca 100644 (file)
--- a/src/jit/flowgraph.cpp
+++ b/src/jit/flowgraph.cpp
@@ -7225,14 +7225,24 @@ bool Compiler::fgAddrCouldBeNull(GenTree* addr)
     return true; // default result: addr could be null
 }
 
-/*****************************************************************************
- *  Optimize the call to the delegate constructor.
- */
+//------------------------------------------------------------------------------
+// fgOptimizeDelegateConstructor: try and optimize construction of a delegate
+//
+// Arguments:
+//    call -- call to original delegate constructor
+//    exactContextHnd -- [out] context handle to update
+//    ldftnToken -- [in]  resolved token for the method the delegate will invoke,
+//      if known, or nullptr if not known
+//
+// Return Value:
+//    Original call tree if no optimization applies.
+//    Updated call tree if optimized.
 
 GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall*            call,
                                                  CORINFO_CONTEXT_HANDLE* ExactContextHnd,
                                                  CORINFO_RESOLVED_TOKEN* ldftnToken)
 {
+    JITDUMP("\nfgOptimizeDelegateConstructor: ");
     noway_assert(call->gtCallType == CT_USER_FUNC);
     CORINFO_METHOD_HANDLE methHnd = call->gtCallMethHnd;
     CORINFO_CLASS_HANDLE  clsHnd  = info.compCompHnd->getMethodClass(methHnd);
@@ -7292,6 +7302,24 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall*            call,
         targetMethodHnd = CORINFO_METHOD_HANDLE(tokenNode->gtIntCon.gtCompileTimeHandle);
     }
 
+    // Verify using the ldftnToken gives us all of what we used to get
+    // via the above pattern match, and more...
+    if (ldftnToken != nullptr)
+    {
+        assert(ldftnToken->hMethod != nullptr);
+
+        if (targetMethodHnd != nullptr)
+        {
+            assert(targetMethodHnd == ldftnToken->hMethod);
+        }
+
+        targetMethodHnd = ldftnToken->hMethod;
+    }
+    else
+    {
+        assert(targetMethodHnd == nullptr);
+    }
+
 #ifdef FEATURE_READYTORUN_COMPILER
     if (opts.IsReadyToRun())
     {
@@ -7299,6 +7327,8 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall*            call,
         {
             if (ldftnToken != nullptr)
             {
+                JITDUMP("optimized\n");
+
                 GenTree*             thisPointer       = call->gtCallObjp;
                 GenTree*             targetObjPointers = call->gtCallArgs->Current();
                 GenTreeArgList*      helperArgs        = nullptr;
@@ -7323,10 +7353,16 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall*            call,
                 call = gtNewHelperCallNode(CORINFO_HELP_READYTORUN_DELEGATE_CTOR, TYP_VOID, helperArgs);
                 call->setEntryPoint(entryPoint);
             }
+            else
+            {
+                JITDUMP("not optimized, CORERT no ldftnToken\n");
+            }
         }
         // ReadyToRun has this optimization for a non-virtual function pointers only for now.
         else if (oper == GT_FTN_ADDR)
         {
+            JITDUMP("optimized\n");
+
             GenTree*        thisPointer       = call->gtCallObjp;
             GenTree*        targetObjPointers = call->gtCallArgs->Current();
             GenTreeArgList* helperArgs        = gtNewArgList(thisPointer, targetObjPointers);
@@ -7338,6 +7374,10 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall*            call,
             assert(!entryPoint.lookupKind.needsRuntimeLookup);
             call->setEntryPoint(entryPoint.constLookup);
         }
+        else
+        {
+            JITDUMP("not optimized, R2R virtual case\n");
+        }
     }
     else
 #endif
@@ -7353,6 +7393,7 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall*            call,
         alternateCtor = info.compCompHnd->GetDelegateCtor(methHnd, clsHnd, targetMethodHnd, &ctorData);
         if (alternateCtor != methHnd)
         {
+            JITDUMP("optimized\n");
             // we erase any inline info that may have been set for generics has it is not needed here,
             // and in fact it will pass the wrong info to the inliner code
             *ExactContextHnd = nullptr;
@@ -7378,6 +7419,14 @@ GenTree* Compiler::fgOptimizeDelegateConstructor(GenTreeCall*            call,
             }
             call->gtCallArgs->Rest()->Rest() = addArgs;
         }
+        else
+        {
+            JITDUMP("not optimized, no alternate ctor\n");
+        }
+    }
+    else
+    {
+        JITDUMP("not optimized, no target method\n");
     }
     return call;
 }

diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
index 905cb97..5a952a4 100644 (file)
--- a/src/jit/importer.cpp
+++ b/src/jit/importer.cpp
@@ -12913,12 +12913,17 @@ void Compiler::impImportBlockCode(BasicBlock* block)
 
             DO_LDFTN:
                 op1 = impMethodPointer(&resolvedToken, &callInfo);
+
                 if (compDonotInline())
                 {
                     return;
                 }
 
+                // Call info may have more precise information about the function than
+                // the resolved token.
                 CORINFO_RESOLVED_TOKEN* heapToken = impAllocateToken(resolvedToken);
+                assert(callInfo.hMethod != nullptr);
+                heapToken->hMethod = callInfo.hMethod;
                 impPushOnStack(op1, typeInfo(heapToken));
 
                 break;
@@ -13025,8 +13030,12 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                 }
 
                 CORINFO_RESOLVED_TOKEN* heapToken = impAllocateToken(resolvedToken);
+
                 assert(heapToken->tokenType == CORINFO_TOKENKIND_Method);
+                assert(callInfo.hMethod != nullptr);
+
                 heapToken->tokenType = CORINFO_TOKENKIND_Ldvirtftn;
+                heapToken->hMethod   = callInfo.hMethod;
                 impPushOnStack(fptr, typeInfo(heapToken));
 
                 break;