From 7ebf9676b4e37c183072f97efaa21e6b4a7d33f3 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 26 Jul 2023 10:42:30 +0200 Subject: [PATCH] Use proper context in initClass for GDV (#87847) Co-authored-by: David Wrighton --- src/coreclr/jit/compiler.h | 13 +++---- src/coreclr/jit/fginline.cpp | 3 +- src/coreclr/jit/importercalls.cpp | 35 ++++++++++++------- src/coreclr/jit/indirectcalltransformer.cpp | 5 +-- .../tools/Common/JitInterface/CorInfoImpl.cs | 32 ++++++++++------- .../superpmi-shared/methodcontext.cpp | 6 ---- 6 files changed, 54 insertions(+), 40 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 31cdad3d3b0399..42581638c95433 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7181,12 +7181,13 @@ class Compiler bool isCompatibleMethodGDV(GenTreeCall* call, CORINFO_METHOD_HANDLE gdvTarget); - void addGuardedDevirtualizationCandidate(GenTreeCall* call, - CORINFO_METHOD_HANDLE methodHandle, - CORINFO_CLASS_HANDLE classHandle, - unsigned methodAttr, - unsigned classAttr, - unsigned likelihood); + void addGuardedDevirtualizationCandidate(GenTreeCall* call, + CORINFO_METHOD_HANDLE methodHandle, + CORINFO_CLASS_HANDLE classHandle, + CORINFO_CONTEXT_HANDLE contextHandle, + unsigned methodAttr, + unsigned classAttr, + unsigned likelihood); int getGDVMaxTypeChecks() { diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 71a0f9985a0688..9fee8f60d87dc7 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -475,7 +475,8 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorimpDevirtualizeCall(call, nullptr, &method, &methodFlags, &context, nullptr, + CORINFO_CONTEXT_HANDLE contextInput = context; + m_compiler->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &contextInput, &context, isLateDevirtualization, explicitTailCall); m_madeChanges = true; } diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 017d5c184451f8..53b0b9fb1058f7 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -5990,8 +5990,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, likelyHood += 100 - likelyHood * numExactClasses; } - addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, exactMethodAttrs, clsAttrs, - likelyHood); + addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, dvInfo.exactContext, exactMethodAttrs, + clsAttrs, likelyHood); } if (call->GetInlineCandidatesCount() == numExactClasses) @@ -6018,6 +6018,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, CORINFO_METHOD_HANDLE likelyMethod = likelyMethodes[candidateId]; unsigned likelihood = likelihoods[candidateId]; + CORINFO_CONTEXT_HANDLE likelyContext = NULL; + uint32_t likelyClassAttribs = 0; if (likelyClass != NO_CLASS_HANDLE) { @@ -6053,7 +6055,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, break; } - likelyMethod = dvInfo.devirtualizedMethod; + likelyContext = dvInfo.exactContext; + likelyMethod = dvInfo.devirtualizedMethod; } uint32_t likelyMethodAttribs = info.compCompHnd->getMethodAttribs(likelyMethod); @@ -6121,8 +6124,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // Add this as a potential candidate. // - addGuardedDevirtualizationCandidate(call, likelyMethod, likelyClass, likelyMethodAttribs, likelyClassAttribs, - likelihood); + addGuardedDevirtualizationCandidate(call, likelyMethod, likelyClass, likelyContext, likelyMethodAttribs, + likelyClassAttribs, likelihood); } } @@ -6147,12 +6150,13 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // classAttr - attributes of the class // likelihood - odds that this class is the class seen at runtime // -void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, - CORINFO_METHOD_HANDLE methodHandle, - CORINFO_CLASS_HANDLE classHandle, - unsigned methodAttr, - unsigned classAttr, - unsigned likelihood) +void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, + CORINFO_METHOD_HANDLE methodHandle, + CORINFO_CLASS_HANDLE classHandle, + CORINFO_CONTEXT_HANDLE contextHandle, + unsigned methodAttr, + unsigned classAttr, + unsigned likelihood) { // This transformation only makes sense for delegate and virtual calls assert(call->IsDelegateInvoke() || call->IsVirtual()); @@ -6225,6 +6229,7 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, pInfo->guardedClassHandle = classHandle; pInfo->likelihood = likelihood; pInfo->requiresInstMethodTableArg = false; + pInfo->exactContextHnd = contextHandle; // If the guarded class is a value class, look for an unboxed entry point. // @@ -6286,8 +6291,14 @@ void Compiler::impMarkInlineCandidate(GenTree* callNode, { InlineResult inlineResult(this, call, nullptr, "impMarkInlineCandidate for GDV"); + CORINFO_CONTEXT_HANDLE moreExactContext = call->GetGDVCandidateInfo(candidateId)->exactContextHnd; + if (moreExactContext == NULL) + { + moreExactContext = exactContextHnd; + } + // Do the actual evaluation - impMarkInlineCandidateHelper(call, candidateId, exactContextHnd, exactContextNeedsRuntimeLookup, callInfo, + impMarkInlineCandidateHelper(call, candidateId, moreExactContext, exactContextNeedsRuntimeLookup, callInfo, ilOffset, &inlineResult); // Ignore non-inlineable candidates // TODO: Consider keeping them to just devirtualize without inlining, at least for interface diff --git a/src/coreclr/jit/indirectcalltransformer.cpp b/src/coreclr/jit/indirectcalltransformer.cpp index 8b0318b935c82b..37f0d626cbbc30 100644 --- a/src/coreclr/jit/indirectcalltransformer.cpp +++ b/src/coreclr/jit/indirectcalltransformer.cpp @@ -862,7 +862,7 @@ class IndirectCallTransformer JITDUMP("Direct call [%06u] in block " FMT_BB "\n", compiler->dspTreeID(call), block->bbNum); - CORINFO_METHOD_HANDLE methodHnd = call->gtCallMethHnd; + CORINFO_METHOD_HANDLE methodHnd = inlineInfo->guardedMethodHandle; CORINFO_CONTEXT_HANDLE context = inlineInfo->exactContextHnd; if (clsHnd != NO_CLASS_HANDLE) { @@ -872,7 +872,8 @@ class IndirectCallTransformer unsigned methodFlags = compiler->info.compCompHnd->getMethodAttribs(methodHnd); const bool isLateDevirtualization = true; const bool explicitTailCall = (call->AsCall()->gtCallMoreFlags & GTF_CALL_M_EXPLICIT_TAILCALL) != 0; - compiler->impDevirtualizeCall(call, nullptr, &methodHnd, &methodFlags, &context, nullptr, + CORINFO_CONTEXT_HANDLE contextInput = context; + compiler->impDevirtualizeCall(call, nullptr, &methodHnd, &methodFlags, &contextInput, &context, isLateDevirtualization, explicitTailCall); } else diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 453e80c381e5c0..4a65778676feb4 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -1277,6 +1277,10 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info) } MethodDesc decl = HandleToObject(info->virtualMethod); + + // Transform from the unboxing thunk to the normal method + decl = decl.IsUnboxingThunk() ? decl.GetUnboxedMethod() : decl; + Debug.Assert(!decl.HasInstantiation); if ((info->context != null) && decl.OwningType.IsInterface) @@ -1369,7 +1373,6 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info) #endif ); } - info->resolvedTokenDevirtualizedUnboxedMethod = default(CORINFO_RESOLVED_TOKEN); } else { @@ -1382,17 +1385,17 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info) , methodWithTokenImpl #endif ); + } - if (unboxingStub) - { - info->resolvedTokenDevirtualizedUnboxedMethod = info->resolvedTokenDevirtualizedMethod; - info->resolvedTokenDevirtualizedUnboxedMethod.tokenContext = contextFromMethod(nonUnboxingImpl); - info->resolvedTokenDevirtualizedUnboxedMethod.hMethod = ObjectToHandle(nonUnboxingImpl); - } - else - { - info->resolvedTokenDevirtualizedUnboxedMethod = default(CORINFO_RESOLVED_TOKEN); - } + if (unboxingStub) + { + info->resolvedTokenDevirtualizedUnboxedMethod = info->resolvedTokenDevirtualizedMethod; + info->resolvedTokenDevirtualizedUnboxedMethod.tokenContext = contextFromMethod(nonUnboxingImpl); + info->resolvedTokenDevirtualizedUnboxedMethod.hMethod = ObjectToHandle(nonUnboxingImpl); + } + else + { + info->resolvedTokenDevirtualizedUnboxedMethod = default(CORINFO_RESOLVED_TOKEN); } #if READYTORUN @@ -1400,8 +1403,11 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info) // Only generate verification for builds with the stress mode enabled if (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout) { - ISymbolNode virtualResolutionNode = _compilation.SymbolNodeFactory.CheckVirtualFunctionOverride(methodWithTokenDecl, objType, methodWithTokenImpl); - AddPrecodeFixup(virtualResolutionNode); + if (!methodWithTokenDecl.Method.OwningType.IsValueType || !methodWithTokenImpl.Method.OwningType.IsValueType) + { + ISymbolNode virtualResolutionNode = _compilation.SymbolNodeFactory.CheckVirtualFunctionOverride(methodWithTokenDecl, objType, methodWithTokenImpl); + AddPrecodeFixup(virtualResolutionNode); + } } #endif info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_SUCCESS; diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index ad0f2c7dd6dae3..3d418e5fe1d3b0 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -1015,12 +1015,6 @@ CorInfoInitClassResult MethodContext::repInitClass(CORINFO_FIELD_HANDLE field, key.method = CastHandle(method); key.context = CastHandle(context); - if ((InitClass == nullptr) || (InitClass->GetIndex(key) == -1)) - { - // We could try additional inlines with stress modes, just reject them. - return CORINFO_INITCLASS_DONT_INLINE; - } - DWORD value = InitClass->Get(key); DEBUG_REP(dmpInitClass(key, value)); CorInfoInitClassResult result = (CorInfoInitClassResult)value;