From e2e1cfa1ce7def86a04c4bc495081d1389fe0c48 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 10 May 2018 17:22:10 -0700 Subject: [PATCH] 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. --- src/jit/flowgraph.cpp | 55 ++++++++++++++++++++++++++++++++++++++++++++++++--- src/jit/importer.cpp | 9 +++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp index be6f5fd..0c7b6ca 100644 --- 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 --- 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; -- 2.7.4