From d00587db256d5684ba7458225c35a06347b487eb Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 22 Jan 2024 14:35:18 +0100 Subject: [PATCH] Late cast expansion: castclass (#97237) --- src/coreclr/jit/helperexpansion.cpp | 30 ++++++++++- src/coreclr/jit/importer.cpp | 78 +---------------------------- 2 files changed, 30 insertions(+), 78 deletions(-) diff --git a/src/coreclr/jit/helperexpansion.cpp b/src/coreclr/jit/helperexpansion.cpp index ad3b20bc7ecc6..61fda46393789 100644 --- a/src/coreclr/jit/helperexpansion.cpp +++ b/src/coreclr/jit/helperexpansion.cpp @@ -1888,9 +1888,8 @@ static CORINFO_CLASS_HANDLE PickLikelyClass(Compiler* comp, IL_OFFSET offset, un // bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt, GenTreeCall* call) { - if (!call->IsHelperCall() || !impIsCastHelperMayHaveProfileData(call->GetHelperNum())) + if (!call->IsHelperCall()) { - // Not a cast helper we're interested in return false; } @@ -1901,6 +1900,26 @@ bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt, return false; } + bool isInstanceOf = false; + switch (call->GetHelperNum()) + { + case CORINFO_HELP_ISINSTANCEOFINTERFACE: + case CORINFO_HELP_ISINSTANCEOFARRAY: + case CORINFO_HELP_ISINSTANCEOFCLASS: + case CORINFO_HELP_ISINSTANCEOFANY: + isInstanceOf = true; + break; + + case CORINFO_HELP_CHKCASTINTERFACE: + case CORINFO_HELP_CHKCASTARRAY: + case CORINFO_HELP_CHKCASTCLASS: + case CORINFO_HELP_CHKCASTANY: + break; + + default: + return false; + } + // Helper calls are never tail calls assert(!call->IsTailCall()); @@ -1946,6 +1965,13 @@ bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt, return false; } + if ((castResult == TypeCompareState::MustNot) && !isInstanceOf) + { + // Don't expand castclass if likelyclass always fails the type check + // it's going to throw an exception anyway. + return false; + } + if ((info.compCompHnd->getClassAttribs(likelyCls) & (CORINFO_FLG_INTERFACE | CORINFO_FLG_ABSTRACT)) != 0) { // Possible scenario: someone changed Foo to be an interface, diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 84e402a85b0ce..701e0463367b8 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -5518,79 +5518,6 @@ GenTree* Compiler::impCastClassOrIsInstToTree( // If the class is exact, the jit can expand the IsInst check inline. canExpandInline = isClassExact; } - - // Check if this cast helper have some profile data - // "isinst" with profile data is moved to a late phase. - // The long-term plan is to move all non-trivial expansions there. - if (impIsCastHelperMayHaveProfileData(helper) && isCastClass) - { - const int maxLikelyClasses = 32; - LikelyClassMethodRecord likelyClasses[maxLikelyClasses]; - unsigned likelyClassCount = - getLikelyClasses(likelyClasses, maxLikelyClasses, fgPgoSchema, fgPgoSchemaCount, fgPgoData, ilOffset); - - if (likelyClassCount > 0) - { -#ifdef DEBUG - for (UINT32 i = 0; i < likelyClassCount; i++) - { - const char* className = eeGetClassName((CORINFO_CLASS_HANDLE)likelyClasses[i].handle); - JITDUMP(" %u) %p (%s) [likelihood:%u%%]\n", i + 1, likelyClasses[i].handle, className, - likelyClasses[i].likelihood); - } - - // Optional stress mode to pick a random known class, rather than - // the most likely known class. - if (JitConfig.JitRandomGuardedDevirtualization() != 0) - { - // Reuse the random inliner's random state. - CLRRandom* const random = - impInlineRoot()->m_inlineStrategy->GetRandom(JitConfig.JitRandomGuardedDevirtualization()); - - unsigned index = static_cast(random->Next(static_cast(likelyClassCount))); - likelyClasses[0].handle = likelyClasses[index].handle; - likelyClasses[0].likelihood = 100; - likelyClassCount = 1; - } -#endif - - LikelyClassMethodRecord likelyClass = likelyClasses[0]; - CORINFO_CLASS_HANDLE likelyCls = (CORINFO_CLASS_HANDLE)likelyClass.handle; - - // if there is a dominating candidate with >= 40% likelihood, use it - const unsigned likelihoodMinThreshold = 40; - if ((likelyCls != NO_CLASS_HANDLE) && (likelyClass.likelihood > likelihoodMinThreshold)) - { - TypeCompareState castResult = - info.compCompHnd->compareTypesForCast(likelyCls, pResolvedToken->hClass); - - // If case of MustNot we still can optimize isinst (only), e.g.: - // - // bool objIsDisposable = obj is IDisposable; - // - // when the profile tells us that obj is mostly Int32, hence, never implements that interface. - // for castclass it makes little sense as it will always throw a cast exception anyway. - if ((castResult == TypeCompareState::Must) || - (castResult == TypeCompareState::MustNot && !isCastClass)) - { - bool isAbstract = (info.compCompHnd->getClassAttribs(likelyCls) & - (CORINFO_FLG_INTERFACE | CORINFO_FLG_ABSTRACT)) != 0; - // If it's abstract it means we most likely deal with a stale PGO data so bail out. - if (!isAbstract) - { - JITDUMP("Adding \"is %s (%X)\" check as a fast path for %s using PGO data.\n", - eeGetClassName(likelyCls), likelyCls, isCastClass ? "castclass" : "isinst"); - - reversedMTCheck = castResult == TypeCompareState::MustNot; - canExpandInline = true; - partialExpand = true; - exactCls = likelyCls; - fastPathLikelihood = likelyClass.likelihood; - } - } - } - } - } } const bool expandInline = canExpandInline && shouldExpandInline; @@ -5621,10 +5548,9 @@ GenTree* Compiler::impCastClassOrIsInstToTree( compCurBB->SetFlags(BBF_HAS_HISTOGRAM_PROFILE); } } - else if (!isCastClass && impIsCastHelperMayHaveProfileData(helper)) + else if (impIsCastHelperMayHaveProfileData(helper)) { - // Maybe the late-cast-expand phase will have a better luck expanding this cast. - // TODO: enable for cast-class as well. + // Leave a note for fgLateCastExpand to expand this helper call call->gtCallMoreFlags |= GTF_CALL_M_CAST_CAN_BE_EXPANDED; call->gtCastHelperILOffset = ilOffset; }