JIT: Drop redundant static initializations from (Equality)Comparer<T>.Default

src/coreclr/jit/compiler.h

@@ -4020,8 +4020,6 @@ class Compiler
                             CORINFO_CALL_INFO* callInfo,
                             IL_OFFSET          rawILOffset);
 
-    CORINFO_CLASS_HANDLE impGetSpecialIntrinsicExactReturnType(CORINFO_METHOD_HANDLE specialIntrinsicHandle);
-
     bool impMethodInfo_hasRetBuffArg(CORINFO_METHOD_INFO* methInfo, CorInfoCallConvExtension callConv);
 
     GenTree* impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HANDLE retClsHnd);
@@ -4069,7 +4067,6 @@ class Compiler
                               var_types             callType,
                               NamedIntrinsic        intrinsicName,
                               bool                  tailCall);
-    NamedIntrinsic lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method);
     GenTree* impUnsupportedNamedIntrinsic(unsigned              helper,
                                           CORINFO_METHOD_HANDLE method,
                                           CORINFO_SIG_INFO*     sig,
@@ -4170,6 +4167,7 @@ class Compiler
         CHECK_SPILL_NONE = -2
     };
 
+    NamedIntrinsic lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method);
     void impBeginTreeList();
     void impEndTreeList(BasicBlock* block, Statement* firstStmt, Statement* lastStmt);
     void impEndTreeList(BasicBlock* block);

src/coreclr/jit/fginline.cpp

@@ -1623,8 +1623,7 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
                 if (argInfo.argHasSideEff)
                 {
                     noway_assert(argInfo.argIsUsed == false);
-                    newStmt     = nullptr;
-                    bool append = true;
+                    newStmt = nullptr;
 
                     if (argNode->gtOper == GT_OBJ || argNode->gtOper == GT_MKREFANY)
                     {
@@ -1633,92 +1632,22 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo)
                         // Just hang the address here in case there are side-effect.
                         newStmt = gtNewStmt(gtUnusedValNode(argNode->AsOp()->gtOp1), callILOffset);
                     }
-                    else
+
+                    // If we don't have something custom to append,
+                    // just append the arg node as an unused value.
+                    if (newStmt == nullptr)
                     {
-                        // In some special cases, unused args with side effects can
-                        // trigger further changes.
-                        //
-                        // (1) If the arg is a static field access and the field access
-                        // was produced by a call to EqualityComparer<T>.get_Default, the
-                        // helper call to ensure the field has a value can be suppressed.
-                        // This helper call is marked as a "Special DCE" helper during
-                        // importation, over in fgGetStaticsCCtorHelper.
-                        //
-                        // (2) NYI. If, after tunneling through GT_RET_VALs, we find that
-                        // the actual arg expression has no side effects, we can skip
-                        // appending all together. This will help jit TP a bit.
-                        //
-                        // Chase through any GT_RET_EXPRs to find the actual argument
-                        // expression.
-                        GenTree* actualArgNode = argNode->gtRetExprVal(&bbFlags);
-
-                        // For case (1)
-                        //
-                        // Look for the following tree shapes
-                        // prejit: (IND (ADD (CONST, CALL(special dce helper...))))
-                        // jit   : (COMMA (CALL(special dce helper...), (FIELD ...)))
-                        if (actualArgNode->gtOper == GT_COMMA)
-                        {
-                            // Look for (COMMA (CALL(special dce helper...), (FIELD ...)))
-                            GenTree* op1 = actualArgNode->AsOp()->gtOp1;
-                            GenTree* op2 = actualArgNode->AsOp()->gtOp2;
-                            if (op1->IsCall() &&
-                                ((op1->AsCall()->gtCallMoreFlags & GTF_CALL_M_HELPER_SPECIAL_DCE) != 0) &&
-                                (op2->gtOper == GT_FIELD) && ((op2->gtFlags & GTF_EXCEPT) == 0))
-                            {
-                                JITDUMP("\nPerforming special dce on unused arg [%06u]:"
-                                        " actual arg [%06u] helper call [%06u]\n",
-                                        argNode->gtTreeID, actualArgNode->gtTreeID, op1->gtTreeID);
-                                // Drop the whole tree
-                                append = false;
-                            }
-                        }
-                        else if (actualArgNode->gtOper == GT_IND)
-                        {
-                            // Look for (IND (ADD (CONST, CALL(special dce helper...))))
-                            GenTree* addr = actualArgNode->AsOp()->gtOp1;
-
-                            if (addr->gtOper == GT_ADD)
-                            {
-                                GenTree* op1 = addr->AsOp()->gtOp1;
-                                GenTree* op2 = addr->AsOp()->gtOp2;
-                                if (op1->IsCall() &&
-                                    ((op1->AsCall()->gtCallMoreFlags & GTF_CALL_M_HELPER_SPECIAL_DCE) != 0) &&
-                                    op2->IsCnsIntOrI())
-                                {
-                                    // Drop the whole tree
-                                    JITDUMP("\nPerforming special dce on unused arg [%06u]:"
-                                            " actual arg [%06u] helper call [%06u]\n",
-                                            argNode->gtTreeID, actualArgNode->gtTreeID, op1->gtTreeID);
-                                    append = false;
-                                }
-                            }
-                        }
+                        newStmt = gtNewStmt(gtUnusedValNode(argNode), callILOffset);
                     }
 
-                    if (!append)
+                    fgInsertStmtAfter(block, afterStmt, newStmt);
+                    afterStmt = newStmt;
+#ifdef DEBUG
+                    if (verbose)
                     {
-                        assert(newStmt == nullptr);
-                        JITDUMP("Arg tree side effects were discardable, not appending anything for arg\n");
+                        gtDispStmt(afterStmt);
                     }
-                    else
-                    {
-                        // If we don't have something custom to append,
-                        // just append the arg node as an unused value.
-                        if (newStmt == nullptr)
-                        {
-                            newStmt = gtNewStmt(gtUnusedValNode(argNode), callILOffset);
-                        }
-
-                        fgInsertStmtAfter(block, afterStmt, newStmt);
-                        afterStmt = newStmt;
-#ifdef DEBUG
-                        if (verbose)
-                        {
-                            gtDispStmt(afterStmt);
-                        }
 #endif // DEBUG
-                    }
                 }
                 else if (argNode->IsBoxedValue())
                 {

src/coreclr/jit/flowgraph.cpp

@@ -910,21 +910,6 @@ GenTreeCall* Compiler::fgGetStaticsCCtorHelper(CORINFO_CLASS_HANDLE cls, CorInfo
 
     GenTreeCall* result = gtNewHelperCallNode(helper, type, argList);
     result->gtFlags |= callFlags;
-
-    // If we're importing the special EqualityComparer<T>.Default or Comparer<T>.Default
-    // intrinsics, flag the helper call. Later during inlining, we can
-    // remove the helper call if the associated field lookup is unused.
-    if ((info.compFlags & CORINFO_FLG_JIT_INTRINSIC) != 0)
-    {
-        NamedIntrinsic ni = lookupNamedIntrinsic(info.compMethodHnd);
-        if ((ni == NI_System_Collections_Generic_EqualityComparer_get_Default) ||
-            (ni == NI_System_Collections_Generic_Comparer_get_Default))
-        {
-            JITDUMP("\nmarking helper call [%06u] as special dce...\n", result->gtTreeID);
-            result->gtCallMoreFlags |= GTF_CALL_M_HELPER_SPECIAL_DCE;
-        }
-    }
-
     return result;
 }
 

src/coreclr/jit/gentree.cpp

@@ -882,6 +882,17 @@ bool GenTreeCall::IsPure(Compiler* compiler) const
 //      true if this call has any side-effects; false otherwise.
 bool GenTreeCall::HasSideEffects(Compiler* compiler, bool ignoreExceptions, bool ignoreCctors) const
 {
+    // Some named intrinsics are known to have ignorable side effects.
+    if (gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)
+    {
+        NamedIntrinsic ni = compiler->lookupNamedIntrinsic(gtCallMethHnd);
+        if ((ni == NI_System_Collections_Generic_Comparer_get_Default) ||
+            (ni == NI_System_Collections_Generic_EqualityComparer_get_Default))
+        {
+            return false;
+        }
+    }
+
     // Generally all GT_CALL nodes are considered to have side-effects, but we may have extra information about helper
     // calls that can prove them side-effect-free.
     if (gtCallType != CT_HELPER)
@@ -15920,9 +15931,10 @@ void Compiler::gtExtractSideEffList(GenTree*  expr,
                 }
 
                 // Generally all GT_CALL nodes are considered to have side-effects.
-                // So if we get here it must be a helper call that we decided it does
+                // So if we get here it must be a helper call or a special intrinsic that we decided it does
                 // not have side effects that we needed to keep.
-                assert(!node->OperIs(GT_CALL) || (node->AsCall()->gtCallType == CT_HELPER));
+                assert(!node->OperIs(GT_CALL) || (node->AsCall()->gtCallType == CT_HELPER) ||
+                       (node->AsCall()->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC));
             }
 
             if ((m_flags & GTF_IS_IN_CSE) != 0)
@@ -17743,13 +17755,50 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
                     break;
                 }
 
-                CORINFO_CLASS_HANDLE specialObjClass = impGetSpecialIntrinsicExactReturnType(call->gtCallMethHnd);
-                if (specialObjClass != nullptr)
+                // Try to get the actual type of [Equality]Comparer<>.Default
+                if ((ni == NI_System_Collections_Generic_Comparer_get_Default) ||
+                    (ni == NI_System_Collections_Generic_EqualityComparer_get_Default))
                 {
-                    objClass    = specialObjClass;
-                    *pIsExact   = true;
-                    *pIsNonNull = true;
-                    break;
+                    CORINFO_SIG_INFO sig;
+                    info.compCompHnd->getMethodSig(call->gtCallMethHnd, &sig);
+                    assert(sig.sigInst.classInstCount == 1);
+                    CORINFO_CLASS_HANDLE typeHnd = sig.sigInst.classInst[0];
+                    assert(typeHnd != nullptr);
+
+                    // Lookup can incorrect when we have __Canon as it won't appear to implement any interface types.
+                    // And if we do not have a final type, devirt & inlining is unlikely to result in much
+                    // simplification. We can use CORINFO_FLG_FINAL to screen out both of these cases.
+                    const DWORD typeAttribs = info.compCompHnd->getClassAttribs(typeHnd);
+                    const bool  isFinalType = ((typeAttribs & CORINFO_FLG_FINAL) != 0);
+
+                    if (isFinalType)
+                    {
+                        if (ni == NI_System_Collections_Generic_EqualityComparer_get_Default)
+                        {
+                            objClass = info.compCompHnd->getDefaultEqualityComparerClass(typeHnd);
+                        }
+                        else
+                        {
+                            assert(ni == NI_System_Collections_Generic_Comparer_get_Default);
+                            objClass = info.compCompHnd->getDefaultComparerClass(typeHnd);
+                        }
+                    }
+
+                    if (objClass == nullptr)
+                    {
+                        // Don't re-visit this intrinsic in this case.
+                        call->gtCallMoreFlags &= ~GTF_CALL_M_SPECIAL_INTRINSIC;
+                        JITDUMP("Special intrinsic for type %s: type not final, so deferring opt\n",
+                                eeGetClassName(typeHnd))
+                    }
+                    else
+                    {
+                        JITDUMP("Special intrinsic for type %s: return type is %s\n", eeGetClassName(typeHnd),
+                                eeGetClassName(objClass))
+                        *pIsExact   = true;
+                        *pIsNonNull = true;
+                        break;
+                    }
                 }
             }
             if (call->IsInlineCandidate())

src/coreclr/jit/gentree.h

@@ -3796,8 +3796,6 @@ enum GenTreeCallFlags : unsigned int
                                                      // to restore real function address and load hidden argument
                                                      // as the first argument for calli. It is CoreRT replacement for instantiating
                                                      // stubs, because executable code cannot be generated at runtime.
-    GTF_CALL_M_HELPER_SPECIAL_DCE      = 0x00020000, // this helper call can be removed if it is part of a comma and
-                                                     // the comma result is unused.
     GTF_CALL_M_DEVIRTUALIZED           = 0x00040000, // this call was devirtualized
     GTF_CALL_M_UNBOXED                 = 0x00080000, // this call was optimized to use the unboxed entry point
     GTF_CALL_M_GUARDED_DEVIRT          = 0x00100000, // this call is a candidate for guarded devirtualization

src/coreclr/jit/importer.cpp

@@ -19254,6 +19254,19 @@ void Compiler::impCheckCanInline(GenTreeCall*           call,
                 goto _exit;
             }
 
+            // It's better for JIT to keep these methods not inlined for CQ.
+            NamedIntrinsic ni;
+            if (pParam->call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)
+            {
+                ni = pParam->pThis->lookupNamedIntrinsic(pParam->call->gtCallMethHnd);
+                if ((ni == NI_System_Collections_Generic_Comparer_get_Default) ||
+                    (ni == NI_System_Collections_Generic_EqualityComparer_get_Default))
+                {
+                    pParam->result->NoteFatal(InlineObservation::CALLEE_SPECIAL_INTRINSIC);
+                    goto _exit;
+                }
+            }
+
             // Speculatively check if initClass() can be done.
             // If it can be done, we will try to inline the method.
             initClassResult =
@@ -21493,79 +21506,6 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
 #endif // FEATURE_READYTORUN_COMPILER
 }
 
-//------------------------------------------------------------------------
-// impGetSpecialIntrinsicExactReturnType: Look for special cases where a call
-//   to an intrinsic returns an exact type
-//
-// Arguments:
-//     methodHnd -- handle for the special intrinsic method
-//
-// Returns:
-//     Exact class handle returned by the intrinsic call, if known.
-//     Nullptr if not known, or not likely to lead to beneficial optimization.
-
-CORINFO_CLASS_HANDLE Compiler::impGetSpecialIntrinsicExactReturnType(CORINFO_METHOD_HANDLE methodHnd)
-{
-    JITDUMP("Special intrinsic: looking for exact type returned by %s\n", eeGetMethodFullName(methodHnd));
-
-    CORINFO_CLASS_HANDLE result = nullptr;
-
-    // See what intrinisc we have...
-    const NamedIntrinsic ni = lookupNamedIntrinsic(methodHnd);
-    switch (ni)
-    {
-        case NI_System_Collections_Generic_Comparer_get_Default:
-        case NI_System_Collections_Generic_EqualityComparer_get_Default:
-        {
-            // Expect one class generic parameter; figure out which it is.
-            CORINFO_SIG_INFO sig;
-            info.compCompHnd->getMethodSig(methodHnd, &sig);
-            assert(sig.sigInst.classInstCount == 1);
-            CORINFO_CLASS_HANDLE typeHnd = sig.sigInst.classInst[0];
-            assert(typeHnd != nullptr);
-
-            // Lookup can incorrect when we have __Canon as it won't appear
-            // to implement any interface types.
-            //
-            // And if we do not have a final type, devirt & inlining is
-            // unlikely to result in much simplification.
-            //
-            // We can use CORINFO_FLG_FINAL to screen out both of these cases.
-            const DWORD typeAttribs = info.compCompHnd->getClassAttribs(typeHnd);
-            const bool  isFinalType = ((typeAttribs & CORINFO_FLG_FINAL) != 0);
-
-            if (isFinalType)
-            {
-                if (ni == NI_System_Collections_Generic_EqualityComparer_get_Default)
-                {
-                    result = info.compCompHnd->getDefaultEqualityComparerClass(typeHnd);
-                }
-                else
-                {
-                    assert(ni == NI_System_Collections_Generic_Comparer_get_Default);
-                    result = info.compCompHnd->getDefaultComparerClass(typeHnd);
-                }
-                JITDUMP("Special intrinsic for type %s: return type is %s\n", eeGetClassName(typeHnd),
-                        result != nullptr ? eeGetClassName(result) : "unknown");
-            }
-            else
-            {
-                JITDUMP("Special intrinsic for type %s: type not final, so deferring opt\n", eeGetClassName(typeHnd));
-            }
-
-            break;
-        }
-
-        default:
-        {
-            JITDUMP("This special intrinsic not handled, sorry...\n");
-            break;
-        }
-    }
-
-    return result;
-}
-
 //------------------------------------------------------------------------
 // impAllocateToken: create CORINFO_RESOLVED_TOKEN into jit-allocated memory and init it.
 //

src/coreclr/jit/inline.def

@@ -57,6 +57,7 @@ INLINE_OBSERVATION(STFLD_NEEDS_HELPER,        bool,   "stfld needs helper",
 INLINE_OBSERVATION(TOO_MANY_ARGUMENTS,        bool,   "too many arguments",                   FATAL,       CALLEE)
 INLINE_OBSERVATION(TOO_MANY_LOCALS,           bool,   "too many locals",                      FATAL,       CALLEE)
 INLINE_OBSERVATION(EXPLICIT_TAIL_PREFIX,      bool,   "explicit tail prefix in callee",       FATAL,       CALLEE)
+INLINE_OBSERVATION(SPECIAL_INTRINSIC,         bool,   "skipped for special intrinsic",        FATAL,       CALLEE)
 
 // ------ Callee Performance -------
 

src/coreclr/jit/valuenum.cpp

@@ -9629,6 +9629,34 @@ void Compiler::fgValueNumberCall(GenTreeCall* call)
     }
     else
     {
+        if (call->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC)
+        {
+            NamedIntrinsic ni = lookupNamedIntrinsic(call->gtCallMethHnd);
+            if ((ni == NI_System_Collections_Generic_Comparer_get_Default) ||
+                (ni == NI_System_Collections_Generic_EqualityComparer_get_Default))
+            {
+                bool                 isExact   = false;
+                bool                 isNotNull = false;
+                CORINFO_CLASS_HANDLE cls       = gtGetClassHandle(call, &isExact, &isNotNull);
+                if ((cls != nullptr) && isExact && isNotNull)
+                {
+                    ValueNum clsVN = vnStore->VNForHandle(ssize_t(cls), GTF_ICON_CLASS_HDL);
+                    ValueNum funcVN;
+                    if (ni == NI_System_Collections_Generic_EqualityComparer_get_Default)
+                    {
+                        funcVN = vnStore->VNForFunc(call->TypeGet(), VNF_GetDefaultEqualityComparer, clsVN);
+                    }
+                    else
+                    {
+                        assert(ni == NI_System_Collections_Generic_Comparer_get_Default);
+                        funcVN = vnStore->VNForFunc(call->TypeGet(), VNF_GetDefaultComparer, clsVN);
+                    }
+                    call->gtVNPair.SetBoth(funcVN);
+                    return;
+                }
+            }
+        }
+
         if (call->TypeGet() == TYP_VOID)
         {
             call->gtVNPair.SetBoth(ValueNumStore::VNForVoid());