diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index a0b08aa3bd1db5..af20430165fac1 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -19198,9 +19198,12 @@ void Compiler::impCheckCanInline(GenTreeCall* call, pInfo = new (pParam->pThis, CMK_Inlining) InlineCandidateInfo; // Null out bits we don't use when we're just inlining - pInfo->guardedClassHandle = nullptr; - pInfo->guardedMethodHandle = nullptr; - pInfo->stubAddr = nullptr; + pInfo->guardedClassHandle = nullptr; + pInfo->guardedMethodHandle = nullptr; + pInfo->guardedMethodUnboxedEntryHandle = nullptr; + pInfo->stubAddr = nullptr; + pInfo->likelihood = 0; + pInfo->requiresInstMethodTableArg = false; } pInfo->methInfo = methInfo; @@ -20338,8 +20341,15 @@ void Compiler::impMarkInlineCandidateHelper(GenTreeCall* call, if (call->IsGuardedDevirtualizationCandidate()) { - fncHandle = call->gtGuardedDevirtualizationCandidateInfo->guardedMethodHandle; - methAttr = info.compCompHnd->getMethodAttribs(fncHandle); + if (call->gtGuardedDevirtualizationCandidateInfo->guardedMethodUnboxedEntryHandle != nullptr) + { + fncHandle = call->gtGuardedDevirtualizationCandidateInfo->guardedMethodUnboxedEntryHandle; + } + else + { + fncHandle = call->gtGuardedDevirtualizationCandidateInfo->guardedMethodHandle; + } + methAttr = info.compCompHnd->getMethodAttribs(fncHandle); } else { @@ -20636,16 +20646,20 @@ bool Compiler::IsMathIntrinsic(GenTree* tree) // code after inlining, if the return value of the inlined call is // the 'this obj' of a subsequent virtual call. // -// If devirtualization succeeds and the call's this object is the -// result of a box, the jit will ask the EE for the unboxed entry -// point. If this exists, the jit will see if it can rework the box -// to instead make a local copy. If that is doable, the call is -// updated to invoke the unboxed entry on the local copy. +// If devirtualization succeeds and the call's this object is a +// (boxed) value type, the jit will ask the EE for the unboxed entry +// point. If this exists, the jit will invoke the unboxed entry +// on the box payload. In addition if the boxing operation is +// visible to the jit and the call is the only consmer of the box, +// the jit will try analyze the box to see if the call can be instead +// instead made on a local copy. If that is doable, the call is +// updated to invoke the unboxed entry on the local copy and the +// boxing operation is removed. // // When guarded devirtualization is enabled, this method will mark // calls as guarded devirtualization candidates, if the type of `this` // is not exactly known, and there is a plausible guess for the type. - +// void Compiler::impDevirtualizeCall(GenTreeCall* call, CORINFO_METHOD_HANDLE* method, unsigned* methodFlags, @@ -20981,126 +20995,236 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, } #endif // defined(DEBUG) - // If the 'this' object is a box, see if we can find the unboxed entry point for the call. - if (thisObj->IsBoxedValue()) + // If the 'this' object is a value class, see if we can rework the call to invoke the + // unboxed entry. This effectively inlines the normally un-inlineable wrapper stub + // and exposes the potentially inlinable unboxed entry method. + // + // We won't optimize explicit tail calls, as ensuring we get the right tail call info + // is tricky (we'd need to pass an updated sig and resolved token back to some callers). + // + if (info.compCompHnd->isValueClass(derivedClass)) { - JITDUMP("Now have direct call to boxed entry point, looking for unboxed entry point\n"); - if (isExplicitTailCall) { - JITDUMP("Call is an explicit tail call, we cannot perform an unbox\n"); - return; + JITDUMP("Have a direct explicit tail call to boxed entry point; can't optimize further\n"); } + else + { + JITDUMP("Have a direct call to boxed entry point. Trying to optimize to call an unboxed entry point\n"); - // Note for some shared methods the unboxed entry point requires an extra parameter. - bool requiresInstMethodTableArg = false; - CORINFO_METHOD_HANDLE unboxedEntryMethod = - info.compCompHnd->getUnboxedEntry(derivedMethod, &requiresInstMethodTableArg); + // Note for some shared methods the unboxed entry point requires an extra parameter. + bool requiresInstMethodTableArg = false; + CORINFO_METHOD_HANDLE unboxedEntryMethod = + info.compCompHnd->getUnboxedEntry(derivedMethod, &requiresInstMethodTableArg); - if (unboxedEntryMethod != nullptr) - { - // Since the call is the only consumer of the box, we know the box can't escape - // since it is being passed an interior pointer. - // - // So, revise the box to simply create a local copy, use the address of that copy - // as the this pointer, and update the entry point to the unboxed entry. - // - // Ideally, we then inline the boxed method and and if it turns out not to modify - // the copy, we can undo the copy too. - if (requiresInstMethodTableArg) + if (unboxedEntryMethod != nullptr) { - // Perform a trial box removal and ask for the type handle tree. - JITDUMP("Unboxed entry needs method table arg...\n"); - GenTree* methodTableArg = gtTryRemoveBoxUpstreamEffects(thisObj, BR_DONT_REMOVE_WANT_TYPE_HANDLE); + bool optimizedTheBox = false; - if (methodTableArg != nullptr) + // If the 'this' object is a local box, see if we can revise things + // to not require boxing. + // + if (thisObj->IsBoxedValue() && !isExplicitTailCall) { - // If that worked, turn the box into a copy to a local var - JITDUMP("Found suitable method table arg tree [%06u]\n", dspTreeID(methodTableArg)); - GenTree* localCopyThis = gtTryRemoveBoxUpstreamEffects(thisObj, BR_MAKE_LOCAL_COPY); - - if (localCopyThis != nullptr) + // Since the call is the only consumer of the box, we know the box can't escape + // since it is being passed an interior pointer. + // + // So, revise the box to simply create a local copy, use the address of that copy + // as the this pointer, and update the entry point to the unboxed entry. + // + // Ideally, we then inline the boxed method and and if it turns out not to modify + // the copy, we can undo the copy too. + if (requiresInstMethodTableArg) { - // Pass the local var as this and the type handle as a new arg - JITDUMP("Success! invoking unboxed entry point on local copy, and passing method table arg\n"); - call->gtCallThisArg = gtNewCallArgs(localCopyThis); - call->gtCallMoreFlags |= GTF_CALL_M_UNBOXED; + // Perform a trial box removal and ask for the type handle tree that fed the box. + // + JITDUMP("Unboxed entry needs method table arg...\n"); + GenTree* methodTableArg = + gtTryRemoveBoxUpstreamEffects(thisObj, BR_DONT_REMOVE_WANT_TYPE_HANDLE); - // Prepend for R2L arg passing or empty L2R passing - if ((Target::g_tgtArgOrder == Target::ARG_ORDER_R2L) || (call->gtCallArgs == nullptr)) + if (methodTableArg != nullptr) { - call->gtCallArgs = gtPrependNewCallArg(methodTableArg, call->gtCallArgs); + // If that worked, turn the box into a copy to a local var + // + JITDUMP("Found suitable method table arg tree [%06u]\n", dspTreeID(methodTableArg)); + GenTree* localCopyThis = gtTryRemoveBoxUpstreamEffects(thisObj, BR_MAKE_LOCAL_COPY); + + if (localCopyThis != nullptr) + { + // Pass the local var as this and the type handle as a new arg + // + JITDUMP("Success! invoking unboxed entry point on local copy, and passing method table " + "arg\n"); + call->gtCallThisArg = gtNewCallArgs(localCopyThis); + call->gtCallMoreFlags |= GTF_CALL_M_UNBOXED; + + // Prepend for R2L arg passing or empty L2R passing + // Append for non-empty L2R + // + if ((Target::g_tgtArgOrder == Target::ARG_ORDER_R2L) || (call->gtCallArgs == nullptr)) + { + call->gtCallArgs = gtPrependNewCallArg(methodTableArg, call->gtCallArgs); + } + else + { + GenTreeCall::Use* beforeArg = call->gtCallArgs; + while (beforeArg->GetNext() != nullptr) + { + beforeArg = beforeArg->GetNext(); + } + + beforeArg->SetNext(gtNewCallArgs(methodTableArg)); + } + + call->gtCallMethHnd = unboxedEntryMethod; + derivedMethod = unboxedEntryMethod; + + // Method attributes will differ because unboxed entry point is shared + // + const DWORD unboxedMethodAttribs = + info.compCompHnd->getMethodAttribs(unboxedEntryMethod); + JITDUMP("Updating method attribs from 0x%08x to 0x%08x\n", derivedMethodAttribs, + unboxedMethodAttribs); + derivedMethodAttribs = unboxedMethodAttribs; + optimizedTheBox = true; + } + else + { + JITDUMP("Sorry, failed to undo the box -- can't convert to local copy\n"); + } } - // Append for non-empty L2R else { - GenTreeCall::Use* beforeArg = call->gtCallArgs; - while (beforeArg->GetNext() != nullptr) - { - beforeArg = beforeArg->GetNext(); - } - - beforeArg->SetNext(gtNewCallArgs(methodTableArg)); + JITDUMP("Sorry, failed to undo the box -- can't find method table arg\n"); } + } + else + { + JITDUMP("Found unboxed entry point, trying to simplify box to a local copy\n"); + GenTree* localCopyThis = gtTryRemoveBoxUpstreamEffects(thisObj, BR_MAKE_LOCAL_COPY); - call->gtCallMethHnd = unboxedEntryMethod; - derivedMethod = unboxedEntryMethod; + if (localCopyThis != nullptr) + { + JITDUMP("Success! invoking unboxed entry point on local copy\n"); + call->gtCallThisArg = gtNewCallArgs(localCopyThis); + call->gtCallMethHnd = unboxedEntryMethod; + call->gtCallMoreFlags |= GTF_CALL_M_UNBOXED; + derivedMethod = unboxedEntryMethod; - // Method attributes will differ because unboxed entry point is shared - const DWORD unboxedMethodAttribs = info.compCompHnd->getMethodAttribs(unboxedEntryMethod); - JITDUMP("Updating method attribs from 0x%08x to 0x%08x\n", derivedMethodAttribs, - unboxedMethodAttribs); - derivedMethodAttribs = unboxedMethodAttribs; + optimizedTheBox = true; + } + else + { + JITDUMP("Sorry, failed to undo the box\n"); + } } - else + + if (optimizedTheBox) { - JITDUMP("Sorry, failed to undo the box -- can't convert to local copy\n"); + +#if FEATURE_TAILCALL_OPT + if (call->IsImplicitTailCall()) + { + JITDUMP("Clearing the implicit tail call flag\n"); + + // If set, we clear the implicit tail call flag + // as we just introduced a new address taken local variable + // + call->gtCallMoreFlags &= ~GTF_CALL_M_IMPLICIT_TAILCALL; + } +#endif // FEATURE_TAILCALL_OPT } } - else - { - JITDUMP("Sorry, failed to undo the box -- can't find method table arg\n"); - } - } - else - { - JITDUMP("Found unboxed entry point, trying to simplify box to a local copy\n"); - GenTree* localCopyThis = gtTryRemoveBoxUpstreamEffects(thisObj, BR_MAKE_LOCAL_COPY); - if (localCopyThis != nullptr) + if (!optimizedTheBox) { - JITDUMP("Success! invoking unboxed entry point on local copy\n"); - call->gtCallThisArg = gtNewCallArgs(localCopyThis); - call->gtCallMethHnd = unboxedEntryMethod; - call->gtCallMoreFlags |= GTF_CALL_M_UNBOXED; - derivedMethod = unboxedEntryMethod; + // If we get here, we have a boxed value class that either wasn't boxed + // locally, or was boxed locally but we were unable to remove the box for + // various reasons. + // + // We may still be able to update the call to invoke the unboxed entry. + // + if (requiresInstMethodTableArg) + { + const DWORD derivedClassAttribs = info.compCompHnd->getClassAttribs(derivedClass); -#if FEATURE_TAILCALL_OPT - if (call->IsImplicitTailCall()) + if ((derivedClassAttribs & CORINFO_FLG_SHAREDINST) != 0) + { + JITDUMP( + "unboxed entry needs MT arg, but the handle we have is shared. Deferring update.\n"); + } + else + { + // See if the method table we have is a viable argument to the unboxed + // entry point. It is, if it's not a shared MT. + // + GenTree* const thisArg = call->gtCallThisArg->GetNode(); + GenTree* const methodTableArg = gtNewIconEmbClsHndNode(derivedClass); + + JITDUMP("revising call to invoke unboxed entry with additional known class handle arg\n"); + + // Update the 'this' pointer to refer to the box payload + // + GenTree* const payloadOffset = gtNewIconNode(TARGET_POINTER_SIZE, TYP_I_IMPL); + GenTree* const boxPayload = gtNewOperNode(GT_ADD, TYP_BYREF, thisArg, payloadOffset); + + call->gtCallThisArg = gtNewCallArgs(boxPayload); + call->gtCallMethHnd = unboxedEntryMethod; + call->gtCallMoreFlags |= GTF_CALL_M_UNBOXED; + + // Method attributes will differ because unboxed entry point is shared + // + const DWORD unboxedMethodAttribs = info.compCompHnd->getMethodAttribs(unboxedEntryMethod); + JITDUMP("Updating method attribs from 0x%08x to 0x%08x\n", derivedMethodAttribs, + unboxedMethodAttribs); + derivedMethod = unboxedEntryMethod; + derivedMethodAttribs = unboxedMethodAttribs; + + // Add the method table argument... + // + // Prepend for R2L arg passing or empty L2R passing + // Append for non-empty L2R + // + if ((Target::g_tgtArgOrder == Target::ARG_ORDER_R2L) || (call->gtCallArgs == nullptr)) + { + call->gtCallArgs = gtPrependNewCallArg(methodTableArg, call->gtCallArgs); + } + else + { + GenTreeCall::Use* beforeArg = call->gtCallArgs; + while (beforeArg->GetNext() != nullptr) + { + beforeArg = beforeArg->GetNext(); + } + + beforeArg->SetNext(gtNewCallArgs(methodTableArg)); + } + } + } + else { - JITDUMP("Clearing the implicit tail call flag\n"); + JITDUMP("revising call to invoke unboxed entry\n"); - // If set, we clear the implicit tail call flag - // as we just introduced a new address taken local variable - // - call->gtCallMoreFlags &= ~GTF_CALL_M_IMPLICIT_TAILCALL; + GenTree* const thisArg = call->gtCallThisArg->GetNode(); + GenTree* const payloadOffset = gtNewIconNode(TARGET_POINTER_SIZE, TYP_I_IMPL); + GenTree* const boxPayload = gtNewOperNode(GT_ADD, TYP_BYREF, thisArg, payloadOffset); + + call->gtCallThisArg = gtNewCallArgs(boxPayload); + call->gtCallMethHnd = unboxedEntryMethod; + call->gtCallMoreFlags |= GTF_CALL_M_UNBOXED; + derivedMethod = unboxedEntryMethod; } -#endif // FEATURE_TAILCALL_OPT - } - else - { - JITDUMP("Sorry, failed to undo the box\n"); } } - } - else - { - // Many of the low-level methods on value classes won't have unboxed entries, - // as they need access to the type of the object. - // - // Note this may be a cue for us to stack allocate the boxed object, since - // we probably know that these objects don't escape. - JITDUMP("Sorry, failed to find unboxed entry point\n"); + else + { + // Many of the low-level methods on value classes won't have unboxed entries, + // as they need access to the type of the object. + // + // Note this may be a cue for us to stack allocate the boxed object, since + // we probably know that these objects don't escape. + JITDUMP("Sorry, failed to find unboxed entry point\n"); + } } } @@ -21514,9 +21638,28 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, // GuardedDevirtualizationCandidateInfo* pInfo = new (this, CMK_Inlining) InlineCandidateInfo; - pInfo->guardedMethodHandle = methodHandle; - pInfo->guardedClassHandle = classHandle; - pInfo->likelihood = likelihood; + pInfo->guardedMethodHandle = methodHandle; + pInfo->guardedMethodUnboxedEntryHandle = nullptr; + pInfo->guardedClassHandle = classHandle; + pInfo->likelihood = likelihood; + pInfo->requiresInstMethodTableArg = false; + + // If the guarded class is a value class, look for an unboxed entry point. + // + if ((classAttr & CORINFO_FLG_VALUECLASS) != 0) + { + JITDUMP(" ... class is a value class, looking for unboxed entry\n"); + bool requiresInstMethodTableArg = false; + CORINFO_METHOD_HANDLE unboxedEntryMethodHandle = + info.compCompHnd->getUnboxedEntry(methodHandle, &requiresInstMethodTableArg); + + if (unboxedEntryMethodHandle != nullptr) + { + JITDUMP(" ... updating GDV candidate with unboxed entry info\n"); + pInfo->guardedMethodUnboxedEntryHandle = unboxedEntryMethodHandle; + pInfo->requiresInstMethodTableArg = requiresInstMethodTableArg; + } + } // Save off the stub address since it shares a union with the candidate info. // diff --git a/src/coreclr/jit/indirectcalltransformer.cpp b/src/coreclr/jit/indirectcalltransformer.cpp index f03482f1109de4..5b6b663e7e035d 100644 --- a/src/coreclr/jit/indirectcalltransformer.cpp +++ b/src/coreclr/jit/indirectcalltransformer.cpp @@ -517,6 +517,7 @@ class IndirectCallTransformer compiler->dspTreeID(origCall), currBlock->bbNum); // We currently need inline candidate info to guarded devirt. + // if (!origCall->IsInlineCandidate()) { JITDUMP("*** %s Bailing on [%06u] -- not an inline candidate\n", Name(), compiler->dspTreeID(origCall)); @@ -745,54 +746,82 @@ class IndirectCallTransformer JITDUMP("Direct call [%06u] in block " FMT_BB "\n", compiler->dspTreeID(call), thenBlock->bbNum); - // Then invoke impDevirtualizeCall to actually - // transform the call for us. It should succeed.... as we have - // now provided an exact typed this. - CORINFO_METHOD_HANDLE methodHnd = inlineInfo->methInfo.ftn; - unsigned methodFlags = inlineInfo->methAttr; + // Then invoke impDevirtualizeCall to actually transform the call for us, + // given the original (base) method and the exact guarded class. It should succeed. + // + CORINFO_METHOD_HANDLE methodHnd = call->gtCallMethHnd; + unsigned methodFlags = compiler->info.compCompHnd->getMethodAttribs(methodHnd); CORINFO_CONTEXT_HANDLE context = inlineInfo->exactContextHnd; const bool isLateDevirtualization = true; - bool explicitTailCall = (call->AsCall()->gtCallMoreFlags & GTF_CALL_M_EXPLICIT_TAILCALL) != 0; + const bool explicitTailCall = (call->AsCall()->gtCallMoreFlags & GTF_CALL_M_EXPLICIT_TAILCALL) != 0; compiler->impDevirtualizeCall(call, &methodHnd, &methodFlags, &context, nullptr, isLateDevirtualization, explicitTailCall); - // Presumably devirt might fail? If so we should try and avoid - // making this a guarded devirt candidate instead of ending - // up here. - assert(!call->IsVirtual()); - - // Re-establish this call as an inline candidate. + // We know this call can devirtualize or we would not have set up GDV here. + // So impDevirtualizeCall should succeed in devirtualizing. // - GenTree* oldRetExpr = inlineInfo->retExpr; - inlineInfo->clsHandle = compiler->info.compCompHnd->getMethodClass(methodHnd); - inlineInfo->exactContextHnd = context; - inlineInfo->preexistingSpillTemp = returnTemp; - call->gtInlineCandidateInfo = inlineInfo; - - // Add the call. - compiler->fgNewStmtAtEnd(thenBlock, call); + assert(!call->IsVirtual()); - // If there was a ret expr for this call, we need to create a new one - // and append it just after the call. + // If the devirtualizer was unable to transform the call to invoke the unboxed entry, the inline info + // we set up may be invalid. We won't be able to inline anyways. So demote the call as an inline candidate. // - // Note the original GT_RET_EXPR is sitting at the join point of the - // guarded expansion and for non-void calls, and now refers to a temp local; - // we set all this up in FixupRetExpr(). - if (oldRetExpr != nullptr) + CORINFO_METHOD_HANDLE unboxedMethodHnd = inlineInfo->guardedMethodUnboxedEntryHandle; + if ((unboxedMethodHnd != nullptr) && (methodHnd != unboxedMethodHnd)) { - GenTree* retExpr = compiler->gtNewInlineCandidateReturnExpr(call, call->TypeGet(), thenBlock->bbFlags); - inlineInfo->retExpr = retExpr; + // Demote this call to a non-inline candidate + // + JITDUMP("Devirtualization was unable to use the unboxed entry; so marking call (to boxed entry) as not " + "inlineable\n"); + + call->gtFlags &= ~GTF_CALL_INLINE_CANDIDATE; + call->gtInlineCandidateInfo = nullptr; if (returnTemp != BAD_VAR_NUM) { - retExpr = compiler->gtNewTempAssign(returnTemp, retExpr); + GenTree* const assign = compiler->gtNewTempAssign(returnTemp, call); + compiler->fgNewStmtAtEnd(thenBlock, assign); } else { - // We should always have a return temp if we return results by value - assert(origCall->TypeGet() == TYP_VOID); + compiler->fgNewStmtAtEnd(thenBlock, call); + } + } + else + { + // Add the call. + // + compiler->fgNewStmtAtEnd(thenBlock, call); + + // Re-establish this call as an inline candidate. + // + GenTree* oldRetExpr = inlineInfo->retExpr; + inlineInfo->clsHandle = compiler->info.compCompHnd->getMethodClass(methodHnd); + inlineInfo->exactContextHnd = context; + inlineInfo->preexistingSpillTemp = returnTemp; + call->gtInlineCandidateInfo = inlineInfo; + + // If there was a ret expr for this call, we need to create a new one + // and append it just after the call. + // + // Note the original GT_RET_EXPR has been bashed to a temp. + // we set all this up in FixupRetExpr(). + if (oldRetExpr != nullptr) + { + GenTree* retExpr = + compiler->gtNewInlineCandidateReturnExpr(call, call->TypeGet(), thenBlock->bbFlags); + inlineInfo->retExpr = retExpr; + + if (returnTemp != BAD_VAR_NUM) + { + retExpr = compiler->gtNewTempAssign(returnTemp, retExpr); + } + else + { + // We should always have a return temp if we return results by value + assert(origCall->TypeGet() == TYP_VOID); + } + compiler->fgNewStmtAtEnd(thenBlock, retExpr); } - compiler->fgNewStmtAtEnd(thenBlock, retExpr); } } diff --git a/src/coreclr/jit/inline.h b/src/coreclr/jit/inline.h index 3ed9a5dc97cf5c..e1fb23e80c7916 100644 --- a/src/coreclr/jit/inline.h +++ b/src/coreclr/jit/inline.h @@ -529,7 +529,9 @@ struct GuardedDevirtualizationCandidateInfo : ClassProfileCandidateInfo { CORINFO_CLASS_HANDLE guardedClassHandle; CORINFO_METHOD_HANDLE guardedMethodHandle; + CORINFO_METHOD_HANDLE guardedMethodUnboxedEntryHandle; unsigned likelihood; + bool requiresInstMethodTableArg; }; // InlineCandidateInfo provides basic information about a particular