JIT: Fold more always failing type checks

src/coreclr/jit/importer.cpp

@@ -5303,70 +5303,83 @@ GenTree* Compiler::impOptimizeCastClassOrIsInst(GenTree* op1, CORINFO_RESOLVED_T
     bool                 isNonNull = false;
     CORINFO_CLASS_HANDLE fromClass = gtGetClassHandle(op1, &isExact, &isNonNull);
 
-    if (fromClass != nullptr)
+    if (fromClass == nullptr)
     {
-        CORINFO_CLASS_HANDLE toClass = pResolvedToken->hClass;
-        JITDUMP("\nConsidering optimization of %s from %s%p (%s) to %p (%s)\n", isCastClass ? "castclass" : "isinst",
-                isExact ? "exact " : "", dspPtr(fromClass), eeGetClassName(fromClass), dspPtr(toClass),
-                eeGetClassName(toClass));
+        JITDUMP("\nCannot optimize since fromClass is unknown\n");
+        return nullptr;
+    }
 
-        // Perhaps we know if the cast will succeed or fail.
-        TypeCompareState castResult = info.compCompHnd->compareTypesForCast(fromClass, toClass);
+    CORINFO_CLASS_HANDLE toClass = pResolvedToken->hClass;
+    JITDUMP("\nConsidering optimization of %s from %s%p (%s) to %p (%s)\n", isCastClass ? "castclass" : "isinst",
+            isExact ? "exact " : "", dspPtr(fromClass), eeGetClassName(fromClass), dspPtr(toClass),
+            eeGetClassName(toClass));
 
-        if (castResult == TypeCompareState::Must)
-        {
-            // Cast will succeed, result is simply op1.
-            JITDUMP("Cast will succeed, optimizing to simply return input\n");
-            return op1;
-        }
-        else if (castResult == TypeCompareState::MustNot)
-        {
-            // See if we can sharpen exactness by looking for final classes
-            if (!isExact)
-            {
-                isExact = impIsClassExact(fromClass);
-            }
+    // Perhaps we know if the cast will succeed or fail.
+    TypeCompareState castResult = info.compCompHnd->compareTypesForCast(fromClass, toClass);
 
-            // Cast to exact type will fail. Handle case where we have
-            // an exact type (that is, fromClass is not a subtype)
-            // and we're not going to throw on failure.
-            if (isExact && !isCastClass)
-            {
-                JITDUMP("Cast will fail, optimizing to return null\n");
+    if (castResult == TypeCompareState::May)
+    {
+        JITDUMP("Result of cast unknown, must generate runtime test\n");
+        return nullptr;
+    }
 
-                // If the cast was fed by a box, we can remove that too.
-                if (op1->IsBoxedValue())
-                {
-                    JITDUMP("Also removing upstream box\n");
-                    gtTryRemoveBoxUpstreamEffects(op1);
-                }
+    if (castResult == TypeCompareState::Must)
+    {
+        // Even if op1 is more derived we know that it will still succeed.
+        JITDUMP("Cast will succeed, optimizing to simply return input\n");
+        return op1;
+    }
 
-                if (gtTreeHasSideEffects(op1, GTF_SIDE_EFFECT))
-                {
-                    impAppendTree(op1, CHECK_SPILL_ALL, impCurStmtDI);
-                }
-                return gtNewNull();
-            }
-            else if (isExact)
-            {
-                JITDUMP("Not optimizing failing castclass (yet)\n");
-            }
-            else
-            {
-                JITDUMP("Can't optimize since fromClass is inexact\n");
-            }
+    assert(castResult == TypeCompareState::MustNot);
+    if (isCastClass)
+    {
+        // TODO-CQ: Throw cast exception inline? Probably not important.
+        JITDUMP("Cannot optimize potentially failing castclass\n");
+        return nullptr;
+    }
+
+    // We know that an object of type fromClass cannot be cast to toClass, but
+    // if it is more derived it could still succeed a cast.
+    if (!isExact)
+    {
+        // We can still fail some type casts if they are between two types that
+        // belong to disjoint type hierarchies.
+        // TODO: compareTypesForCast(interfaceType, otherType) returning
+        // MustNot seems less useful than just having it return May. If we
+        // change this on EE side we can avoid these getClassAttribs calls.
+        if ((info.compCompHnd->getClassAttribs(fromClass) & CORINFO_FLG_INTERFACE) != 0)
+        {
+            JITDUMP("Cannot optimize potentially failing isinst from interface type\n");
+            return nullptr;
         }
-        else
+
+        if ((info.compCompHnd->getClassAttribs(toClass) & CORINFO_FLG_INTERFACE) != 0)
         {
-            JITDUMP("Result of cast unknown, must generate runtime test\n");
+            JITDUMP("Cannot optimize potentially failing isinst to interface type\n");
+            return nullptr;
+        }
+
+        if (info.compCompHnd->compareTypesForCast(toClass, fromClass) != TypeCompareState::MustNot)
+        {
+            JITDUMP("Cannot optimize potentially failing isinst: derived type cast may succeed\n");
+            return nullptr;
         }
     }
-    else
+
+    JITDUMP("Cast will fail, optimizing to return null\n");
+
+    // If the cast was fed by a box, we can remove that too.
+    if (op1->IsBoxedValue())
     {
-        JITDUMP("\nCan't optimize since fromClass is unknown\n");
+        JITDUMP("Also removing upstream box\n");
+        gtTryRemoveBoxUpstreamEffects(op1);
     }
 
-    return nullptr;
+    if (gtTreeHasSideEffects(op1, GTF_SIDE_EFFECT))
+    {
+        impAppendTree(op1, CHECK_SPILL_ALL, impCurStmtDI);
+    }
+    return gtNewNull();
 }
 
 //------------------------------------------------------------------------