Revert "Optimize BOX+UNBOX for "T? -> T" and "T -> T?" (#104931)" (#105515)

* Revert "Optimize BOX+UNBOX for "T? -> T" and "T -> T?" (#104931)"

This reverts commit 9df4f7f.

* Add back eeIsSharedInst

Author: ericstj

src/coreclr/jit/compiler.h

@@ -3468,7 +3468,7 @@ class Compiler
 
     GenTree* gtNewMustThrowException(unsigned helper, var_types type, CORINFO_CLASS_HANDLE clsHnd);
 
-    GenTreeLclFld* gtNewLclFldNode(unsigned lnum, var_types type, unsigned offset, ClassLayout* layout = nullptr);
+    GenTreeLclFld* gtNewLclFldNode(unsigned lnum, var_types type, unsigned offset);
     GenTreeRetExpr* gtNewInlineCandidateReturnExpr(GenTreeCall* inlineCandidate, var_types type);
 
     GenTreeFieldAddr* gtNewFieldAddrNode(var_types            type,
@@ -4512,10 +4512,7 @@ class Compiler
         MakeInlineObservation = 2,
     };
 
-    GenTree* impStoreNullableFields(CORINFO_CLASS_HANDLE nullableCls,
-        GenTree* value);
     GenTree* impInlineUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenTree* nullableClsNode, GenTree* obj);
-    void impLoadNullableFields(GenTree* nullableObj, CORINFO_CLASS_HANDLE nullableCls, GenTree** hasValueFld, GenTree** valueFld);
 
     int impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken,
                            const BYTE*             codeAddr,

src/coreclr/jit/gentree.cpp

@@ -8284,9 +8284,9 @@ GenTreeConditional* Compiler::gtNewConditionalNode(
     return node;
 }
 
-GenTreeLclFld* Compiler::gtNewLclFldNode(unsigned lnum, var_types type, unsigned offset, ClassLayout* layout)
+GenTreeLclFld* Compiler::gtNewLclFldNode(unsigned lnum, var_types type, unsigned offset)
 {
-    GenTreeLclFld* node = new (this, GT_LCL_FLD) GenTreeLclFld(GT_LCL_FLD, type, lnum, offset, layout);
+    GenTreeLclFld* node = new (this, GT_LCL_FLD) GenTreeLclFld(GT_LCL_FLD, type, lnum, offset, nullptr);
     return node;
 }
 
@@ -15073,7 +15073,8 @@ GenTree* Compiler::gtOptimizeEnumHasFlag(GenTree* thisOp, GenTree* flagOp)
 
     // If we have a shared type instance we can't safely check type
     // equality, so bail.
-    if (eeIsSharedInst(thisHnd))
+    DWORD classAttribs = info.compCompHnd->getClassAttribs(thisHnd);
+    if (classAttribs & CORINFO_FLG_SHAREDINST)
     {
         JITDUMP("bailing, have shared instance type\n");
         return nullptr;
@@ -18884,7 +18885,8 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
                 // processing the call, but we could/should apply
                 // similar sharpening to the argument and local types
                 // of the inlinee.
-                if (eeIsSharedInst(objClass))
+                const unsigned retClassFlags = info.compCompHnd->getClassAttribs(objClass);
+                if (retClassFlags & CORINFO_FLG_SHAREDINST)
                 {
                     CORINFO_CONTEXT_HANDLE context = inlInfo->exactContextHnd;
 

src/coreclr/jit/importer.cpp

@@ -2979,84 +2979,6 @@ GenTree* Compiler::impInlineUnboxNullable(CORINFO_CLASS_HANDLE nullableCls, GenT
     return gtNewLclvNode(resultTmp, TYP_STRUCT);
 }
 
-//------------------------------------------------------------------------
-// impStoreNullableFields: create a Nullable<T> object and store
-//    'hasValue' (always true) and the given value for 'value' field
-//
-// Arguments:
-//    nullableCls - class handle for the Nullable<T> class
-//    value       - value to store in 'value' field
-//
-// Return Value:
-//    A local node representing the created Nullable<T> object
-//
-GenTree* Compiler::impStoreNullableFields(CORINFO_CLASS_HANDLE nullableCls, GenTree* value)
-{
-    assert(info.compCompHnd->isNullableType(nullableCls) == TypeCompareState::Must);
-
-    CORINFO_FIELD_HANDLE valueFldHnd = info.compCompHnd->getFieldInClass(nullableCls, 1);
-    CORINFO_CLASS_HANDLE valueStructCls;
-    var_types            valueType = JITtype2varType(info.compCompHnd->getFieldType(valueFldHnd, &valueStructCls));
-
-    // We still make some assumptions about the layout of Nullable<T> in JIT
-    static_assert_no_msg(OFFSETOF__CORINFO_NullableOfT__hasValue == 0);
-    unsigned hasValOffset = OFFSETOF__CORINFO_NullableOfT__hasValue;
-    unsigned valueOffset  = info.compCompHnd->getFieldOffset(valueFldHnd);
-
-    // Define the resulting Nullable<T> local
-    unsigned resultTmp = lvaGrabTemp(true DEBUGARG("Nullable<T> tmp"));
-    lvaSetStruct(resultTmp, nullableCls, false);
-
-    // Now do two stores:
-    GenTree*     hasValueStore = gtNewStoreLclFldNode(resultTmp, TYP_UBYTE, hasValOffset, gtNewIconNode(1));
-    ClassLayout* layout        = valueType == TYP_STRUCT ? typGetObjLayout(valueStructCls) : nullptr;
-    GenTree*     valueStore    = gtNewStoreLclFldNode(resultTmp, valueType, layout, valueOffset, value);
-
-    impAppendTree(hasValueStore, CHECK_SPILL_ALL, impCurStmtDI);
-    impAppendTree(valueStore, CHECK_SPILL_ALL, impCurStmtDI);
-    return gtNewLclvNode(resultTmp, TYP_STRUCT);
-}
-
-//------------------------------------------------------------------------
-// impLoadNullableFields: get 'hasValue' and 'value' field loads for Nullable<T> object
-//
-// Arguments:
-//    nullableObj - tree representing the Nullable<T> object
-//    nullableCls - class handle for the Nullable<T> class
-//    hasValueFld - pointer to store the 'hasValue' field load tree
-//    valueFld    - pointer to store the 'value' field load tree
-//
-void Compiler::impLoadNullableFields(GenTree*             nullableObj,
-                                     CORINFO_CLASS_HANDLE nullableCls,
-                                     GenTree**            hasValueFld,
-                                     GenTree**            valueFld)
-{
-    assert(info.compCompHnd->isNullableType(nullableCls) == TypeCompareState::Must);
-
-    CORINFO_FIELD_HANDLE valueFldHnd = info.compCompHnd->getFieldInClass(nullableCls, 1);
-    CORINFO_CLASS_HANDLE valueStructCls;
-    var_types            valueType   = JITtype2varType(info.compCompHnd->getFieldType(valueFldHnd, &valueStructCls));
-    ClassLayout*         valueLayout = valueType == TYP_STRUCT ? typGetObjLayout(valueStructCls) : nullptr;
-
-    static_assert_no_msg(OFFSETOF__CORINFO_NullableOfT__hasValue == 0);
-    unsigned hasValOffset = OFFSETOF__CORINFO_NullableOfT__hasValue;
-    unsigned valueOffset  = info.compCompHnd->getFieldOffset(valueFldHnd);
-
-    unsigned objTmp;
-    if (!nullableObj->OperIs(GT_LCL_VAR))
-    {
-        objTmp = lvaGrabTemp(true DEBUGARG("Nullable<T> tmp"));
-        impStoreToTemp(objTmp, nullableObj, CHECK_SPILL_ALL);
-    }
-    else
-    {
-        objTmp = nullableObj->AsLclVarCommon()->GetLclNum();
-    }
-
-    *hasValueFld = gtNewLclFldNode(objTmp, TYP_UBYTE, hasValOffset);
-    *valueFld    = gtNewLclFldNode(objTmp, valueType, valueOffset, valueLayout);
-}
-
 //------------------------------------------------------------------------
 // impBoxPatternMatch: match and import common box idioms
 //
@@ -3122,40 +3044,6 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken,
                     {
                         optimize = true;
                     }
-                    //
-                    // Also, try to optimize (T)(object)nullableT
-                    //
-                    else if (!eeIsSharedInst(unboxResolvedToken.hClass) &&
-                             (info.compCompHnd->isNullableType(pResolvedToken->hClass) == TypeCompareState::Must) &&
-                             (info.compCompHnd->getTypeForBox(pResolvedToken->hClass) == unboxResolvedToken.hClass))
-                    {
-                        GenTree* hasValueFldTree;
-                        GenTree* valueFldTree;
-                        impLoadNullableFields(impPopStack().val, pResolvedToken->hClass, &hasValueFldTree,
-                                              &valueFldTree);
-
-                        // Push "hasValue == 0 ? throw new NullReferenceException() : NOP" qmark
-                        GenTree*      fallback = gtNewHelperCallNode(CORINFO_HELP_THROWNULLREF, TYP_VOID);
-                        GenTree*      cond     = gtNewOperNode(GT_EQ, TYP_INT, hasValueFldTree, gtNewIconNode(0));
-                        GenTreeColon* colon    = gtNewColonNode(TYP_VOID, fallback, gtNewNothingNode());
-                        GenTree*      qmark    = gtNewQmarkNode(TYP_VOID, cond, colon);
-                        impAppendTree(qmark, CHECK_SPILL_ALL, impCurStmtDI);
-
-                        // Now push the value field
-                        impPushOnStack(valueFldTree, typeInfo(valueFldTree->TypeGet()));
-                        optimize = true;
-                    }
-                    //
-                    // Vice versa, try to optimize (T?)(object)nonNullableT
-                    //
-                    else if (!eeIsSharedInst(pResolvedToken->hClass) &&
-                             (info.compCompHnd->isNullableType(unboxResolvedToken.hClass) == TypeCompareState::Must) &&
-                             (info.compCompHnd->getTypeForBox(unboxResolvedToken.hClass) == pResolvedToken->hClass))
-                    {
-                        GenTree* result = impStoreNullableFields(unboxResolvedToken.hClass, impPopStack().val);
-                        impPushOnStack(result, typeInfo(result->TypeGet()));
-                        optimize = true;
-                    }
                 }
 
                 if (optimize)
@@ -5814,7 +5702,7 @@ GenTree* Compiler::impCastClassOrIsInstToTree(GenTree*                op1,
     // We can convert constant-ish tokens of nullable to its underlying type.
     // However, when the type is shared generic parameter like Nullable<Struct<__Canon>>, the actual type will require
     // runtime lookup. It's too complex to add another level of indirection in op2, fallback to the cast helper instead.
-    if (isClassExact && !eeIsSharedInst(pResolvedToken->hClass))
+    if (isClassExact && !(info.compCompHnd->getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST))
     {
         CORINFO_CLASS_HANDLE hClass = info.compCompHnd->getTypeForBox(pResolvedToken->hClass);
         if (hClass != pResolvedToken->hClass)
@@ -5860,7 +5748,7 @@ GenTree* Compiler::impCastClassOrIsInstToTree(GenTree*                op1,
             !compCurBB->isRunRarely())
         {
             // It doesn't make sense to instrument "x is T" or "(T)x" for shared T
-            if (!eeIsSharedInst(pResolvedToken->hClass))
+            if ((info.compCompHnd->getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST) == 0)
             {
                 HandleHistogramProfileCandidateInfo* pInfo =
                     new (this, CMK_Inlining) HandleHistogramProfileCandidateInfo;
@@ -9736,7 +9624,8 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                             CORINFO_FIELD_INFO fi;
                             eeGetFieldInfo(&fldToken, CORINFO_ACCESS_SET, &fi);
                             unsigned flagsToCheck = CORINFO_FLG_FIELD_STATIC | CORINFO_FLG_FIELD_FINAL;
-                            if (((fi.fieldFlags & flagsToCheck) == flagsToCheck) && !eeIsSharedInst(info.compClassHnd))
+                            if (((fi.fieldFlags & flagsToCheck) == flagsToCheck) &&
+                                ((info.compCompHnd->getClassAttribs(info.compClassHnd) & CORINFO_FLG_SHAREDINST) == 0))
                             {
 #ifdef FEATURE_READYTORUN
                                 if (opts.IsReadyToRun())

src/coreclr/jit/valuenum.cpp

@@ -12255,7 +12255,7 @@ void Compiler::fgValueNumberTree(GenTree* tree)
                     {
                         JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle));
                         // Filter out all shared generic instantiations
-                        if (!eeIsSharedInst(handle))
+                        if ((info.compCompHnd->getClassAttribs(handle) & CORINFO_FLG_SHAREDINST) == 0)
                         {
                             void* pEmbedClsHnd;
                             void* embedClsHnd = (void*)info.compCompHnd->embedClassHandle(handle, &pEmbedClsHnd);