Late cast expansion: castclass (dotnet#97237)

Author: EgorBo

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,

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<unsigned>(random->Next(static_cast<int>(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;
         }