Disable JitDoOldStructRetyping by default. (#37745)

* Disable retyping by default.

* Keep block init/copy as baseline.

Total bytes of diff: -21971 (-0.07% of base)
3075 total methods with Code Size differences (1589 improved, 1486 regressed), 184523 unchanged.

Note: it improves code with retyping as well:
808 total methods with Code Size differences (808 improved, 0 regressed), 186790 unchanged.
Found 55 files with textual diffs.
Crossgen CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies for  default jit
Summary of Code Size diffs:
(Lower is better)
Total bytes of diff: -22923 (-0.07% of base)

* Don't mark LCL_VAR that is used in RETURN(IND(ADDR(LCL_VAR)) as address taken when possible.

Protect against a promoted struct with a hole like struct<8> {hole 4; int a;};

* Replace 1-field structs with the field for returns.

* Add SSA support.

* Review response.

* isOpaqueSIMDLclVar fix

* Add tests for structs with independently promoted SIMD fields.

* Old retyping fix.

* Don't try to replace SIMD fields.

src/coreclr/src/jit/codegenarmarch.cpp

@@ -1108,7 +1108,7 @@ void CodeGen::genCodeForBitCast(GenTreeOp* treeNode)
         JITDUMP("Changing type of BITCAST source to load directly.");
         genCodeForTreeNode(op1);
     }
-    else if (varTypeIsFloating(treeNode) != varTypeIsFloating(op1))
+    else if (varTypeUsesFloatReg(treeNode) != varTypeUsesFloatReg(op1))
     {
         regNumber srcReg = op1->GetRegNum();
         assert(genTypeSize(op1->TypeGet()) == genTypeSize(targetType));

src/coreclr/src/jit/compiler.h

@@ -985,6 +985,8 @@ class LclVarDsc
         return GetLayout()->GetRegisterType();
     }
 
+    bool CanBeReplacedWithItsField(Compiler* comp) const;
+
 #ifdef DEBUG
 public:
     const char* lvReason;
@@ -5608,6 +5610,7 @@ class Compiler
     GenTree* fgMorphCopyBlock(GenTree* tree);
     GenTree* fgMorphForRegisterFP(GenTree* tree);
     GenTree* fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac = nullptr);
+    GenTree* fgMorphRetInd(GenTreeUnOp* tree);
     GenTree* fgMorphModToSubMulDiv(GenTreeOp* tree);
     GenTree* fgMorphSmpOpOptional(GenTreeOp* tree);
 
@@ -7977,7 +7980,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
     // vector type (i.e. do not analyze or promote its fields).
     // Note that all but the fixed vector types are opaque, even though they may
     // actually be declared as having fields.
-    bool isOpaqueSIMDType(CORINFO_CLASS_HANDLE structHandle)
+    bool isOpaqueSIMDType(CORINFO_CLASS_HANDLE structHandle) const
     {
         return ((m_simdHandleCache != nullptr) && (structHandle != m_simdHandleCache->SIMDVector2Handle) &&
                 (structHandle != m_simdHandleCache->SIMDVector3Handle) &&
@@ -7993,7 +7996,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
     }
 
     // Returns true if the lclVar is an opaque SIMD type.
-    bool isOpaqueSIMDLclVar(LclVarDsc* varDsc)
+    bool isOpaqueSIMDLclVar(const LclVarDsc* varDsc) const
     {
         if (!varDsc->lvSIMDType)
         {
@@ -9185,7 +9188,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 
     // Returns true if the method returns a value in more than one return register,
     // it should replace/be  merged with compMethodReturnsMultiRegRetType when #36868 is fixed.
-    // The difference from original `compMethodReturnsMultiRegRetType` is in ARM64 SIMD16 handling,
+    // The difference from original `compMethodReturnsMultiRegRetType` is in ARM64 SIMD* handling,
     // this method correctly returns false for it (it is passed as HVA), when the original returns true.
     bool compMethodReturnsMultiRegRegTypeAlternate()
     {
@@ -9195,8 +9198,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
         return varTypeIsLong(info.compRetNativeType);
 #else // targets: X64-UNIX, ARM64 or ARM32
 #if defined(TARGET_ARM64)
-        // TYP_SIMD16 is returned in one register.
-        if (info.compRetNativeType == TYP_SIMD16)
+        // TYP_SIMD* are returned in one register.
+        if (varTypeIsSIMD(info.compRetNativeType))
         {
             return false;
         }

src/coreclr/src/jit/flowgraph.cpp

@@ -89,7 +89,8 @@ void Compiler::fgInit()
     fgCurBBEpochSize         = 0;
     fgBBSetCountInSizeTUnits = 0;
 
-    genReturnBB = nullptr;
+    genReturnBB    = nullptr;
+    genReturnLocal = BAD_VAR_NUM;
 
     /* We haven't reached the global morphing phase */
     fgGlobalMorph = false;

src/coreclr/src/jit/jitconfigvalues.h

@@ -439,7 +439,7 @@ CONFIG_INTEGER(JitSaveFpLrWithCalleeSavedRegisters, W("JitSaveFpLrWithCalleeSave
 #endif // defined(TARGET_ARM64)
 #endif // DEBUG
 
-CONFIG_INTEGER(JitDoOldStructRetyping, W("JitDoOldStructRetyping"), 1) // Allow Jit to retype structs as primitive types
+CONFIG_INTEGER(JitDoOldStructRetyping, W("JitDoOldStructRetyping"), 0) // Allow Jit to retype structs as primitive types
                                                                        // when possible.
 
 #undef CONFIG_INTEGER

src/coreclr/src/jit/lclmorph.cpp

@@ -521,6 +521,37 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
                 PopValue();
                 break;
 
+            case GT_RETURN:
+                if (TopValue(0).Node() != node)
+                {
+                    assert(TopValue(1).Node() == node);
+                    assert(TopValue(0).Node() == node->gtGetOp1());
+                    GenTreeUnOp* ret    = node->AsUnOp();
+                    GenTree*     retVal = ret->gtGetOp1();
+                    if (!m_compiler->compDoOldStructRetyping() && retVal->OperIs(GT_LCL_VAR))
+                    {
+                        // TODO-1stClassStructs: this block is a temporary workaround to keep diffs small,
+                        // having `doNotEnreg` affect block init and copy transformations that affect many methods.
+                        // I have a change that introduces more precise and effective solution for that, but it would
+                        // be merged separatly.
+                        GenTreeLclVar* lclVar = retVal->AsLclVar();
+                        unsigned       lclNum = lclVar->GetLclNum();
+                        if (!m_compiler->compMethodReturnsMultiRegRegTypeAlternate() &&
+                            !m_compiler->lvaIsImplicitByRefLocal(lclVar->GetLclNum()))
+                        {
+                            LclVarDsc* varDsc = m_compiler->lvaGetDesc(lclNum);
+                            if (varDsc->lvFieldCnt > 1)
+                            {
+                                m_compiler->lvaSetVarDoNotEnregister(lclNum DEBUGARG(Compiler::DNER_BlockOp));
+                            }
+                        }
+                    }
+
+                    EscapeValue(TopValue(0), node);
+                    PopValue();
+                }
+                break;
+
             default:
                 while (TopValue(0).Node() != node)
                 {

src/coreclr/src/jit/lclvars.cpp

@@ -1974,6 +1974,12 @@ bool Compiler::StructPromotionHelper::ShouldPromoteStructVar(unsigned lclNum)
             shouldPromote = false;
         }
     }
+    else if (!compiler->compDoOldStructRetyping() && (lclNum == compiler->genReturnLocal) &&
+             (structPromotionInfo.fieldCnt > 1))
+    {
+        // TODO-1stClassStructs: a temporary solution to keep diffs small, it will be fixed later.
+        shouldPromote = false;
+    }
 
     //
     // If the lvRefCnt is zero and we have a struct promoted parameter we can end up with an extra store of
@@ -3640,6 +3646,53 @@ var_types LclVarDsc::lvaArgType()
     return type;
 }
 
+//----------------------------------------------------------------------------------------------
+// CanBeReplacedWithItsField: check if a whole struct reference could be replaced by a field.
+//
+// Arguments:
+//    comp - the compiler instance;
+//
+// Return Value:
+//    true if that can be replaced, false otherwise.
+//
+// Notes:
+//    The replacement can be made only for independently promoted structs
+//    with 1 field without holes.
+//
+bool LclVarDsc::CanBeReplacedWithItsField(Compiler* comp) const
+{
+    if (!lvPromoted)
+    {
+        return false;
+    }
+
+    if (comp->lvaGetPromotionType(this) != Compiler::PROMOTION_TYPE_INDEPENDENT)
+    {
+        return false;
+    }
+    if (lvFieldCnt != 1)
+    {
+        return false;
+    }
+    if (lvContainsHoles)
+    {
+        return false;
+    }
+
+#if defined(FEATURE_SIMD)
+    // If we return `struct A { SIMD16 a; }` we split the struct into several fields.
+    // In order to do that we have to have its field `a` in memory. Right now lowering cannot
+    // handle RETURN struct(multiple registers)->SIMD16(one register), but it can be improved.
+    LclVarDsc* fieldDsc = comp->lvaGetDesc(lvFieldLclStart);
+    if (varTypeIsSIMD(fieldDsc))
+    {
+        return false;
+    }
+#endif // FEATURE_SIMD
+
+    return true;
+}
+
 //------------------------------------------------------------------------
 // lvaMarkLclRefs: increment local var references counts and more
 //