Delete `JitDoOldStructRetyping` artifacts. (#51092)

* Delete `JitDoOldStructRetyping`.

* delete unnecessary spilling in `fgUpdateInlineReturnExpressionPlaceHolder`.

src/coreclr/jit/compiler.h

@@ -5848,7 +5848,6 @@ private:
                                      unsigned             argIndex,
                                      CORINFO_CLASS_HANDLE copyBlkClass);
 
-    void fgFixupStructReturn(GenTree* call);
     GenTree* fgMorphLocalVar(GenTree* tree, bool forceRemorph);
 
 public:
@@ -9617,11 +9616,6 @@ public:
 #endif // TARGET_AMD64
     }
 
-    bool compDoOldStructRetyping()
-    {
-        return JitConfig.JitDoOldStructRetyping();
-    }
-
     // Returns true if the method returns a value in more than one return register
     // TODO-ARM-Bug: Deal with multi-register genReturnLocaled structs?
     // TODO-ARM64: Does this apply for ARM64 too?

src/coreclr/jit/fgbasic.cpp

@@ -2560,14 +2560,7 @@ void Compiler::fgFindBasicBlocks()
             {
                 // The lifetime of this var might expand multiple BBs. So it is a long lifetime compiler temp.
                 lvaInlineeReturnSpillTemp = lvaGrabTemp(false DEBUGARG("Inline return value spill temp"));
-                if (compDoOldStructRetyping())
-                {
-                    lvaTable[lvaInlineeReturnSpillTemp].lvType = info.compRetNativeType;
-                }
-                else
-                {
-                    lvaTable[lvaInlineeReturnSpillTemp].lvType = info.compRetType;
-                }
+                lvaTable[lvaInlineeReturnSpillTemp].lvType = info.compRetType;
 
                 // If the method returns a ref class, set the class of the spill temp
                 // to the method's return value. We may update this later if it turns

src/coreclr/jit/fginline.cpp

@@ -618,56 +618,8 @@ Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTr
 #endif // FEATURE_MULTIREG_RET
 
             case SPK_EnclosingType:
-            {
-                // For enclosing type returns, we must return the call value to a temp since
-                // the return type is larger than the struct type.
-                if (!tree->IsCall())
-                {
-                    break;
-                }
-
-                GenTreeCall* call = tree->AsCall();
-
-                assert(call->gtReturnType == TYP_STRUCT);
-
-                if (call->gtReturnType != TYP_STRUCT)
-                {
-                    break;
-                }
-
-                JITDUMP("\nCall returns small struct via enclosing type, retyping. Before:\n");
-                DISPTREE(call);
-
-                // Create new struct typed temp for return value
-                const unsigned tmpNum =
-                    comp->lvaGrabTemp(true DEBUGARG("small struct return temp for rejected inline"));
-                comp->lvaSetStruct(tmpNum, retClsHnd, false);
-                GenTree* assign = comp->gtNewTempAssign(tmpNum, call);
-
-                // Modify assign tree and call return types to the primitive return type
-                call->gtReturnType = returnType;
-                call->gtType       = returnType;
-                assign->gtType     = returnType;
-
-                // Modify the temp reference in the assign as a primitive reference via GT_LCL_FLD
-                GenTree* tempAsPrimitive = assign->AsOp()->gtOp1;
-                assert(tempAsPrimitive->gtOper == GT_LCL_VAR);
-                tempAsPrimitive->gtType = returnType;
-                tempAsPrimitive->ChangeOper(GT_LCL_FLD);
-
-                // Return temp as value of call tree via comma
-                GenTree* tempAsStruct = comp->gtNewLclvNode(tmpNum, TYP_STRUCT);
-                GenTree* comma        = comp->gtNewOperNode(GT_COMMA, TYP_STRUCT, assign, tempAsStruct);
-                parent->ReplaceOperand(pTree, comma);
-
-                JITDUMP("\nAfter:\n");
-                DISPTREE(comma);
-            }
-            break;
-
             case SPK_PrimitiveType:
-                // We should have already retyped the call as a primitive type
-                // when we first imported the call
+                // No work needs to be done, the call has struct type and should keep it.
                 break;
 
             case SPK_ByReference:

src/coreclr/jit/flowgraph.cpp

@@ -2317,17 +2317,10 @@ private:
 
             if (comp->compMethodReturnsNativeScalarType())
             {
-                if (!comp->compDoOldStructRetyping())
+                returnLocalDsc.lvType = genActualType(comp->info.compRetType);
+                if (varTypeIsStruct(returnLocalDsc.lvType))
                 {
-                    returnLocalDsc.lvType = genActualType(comp->info.compRetType);
-                    if (varTypeIsStruct(returnLocalDsc.lvType))
-                    {
-                        comp->lvaSetStruct(returnLocalNum, comp->info.compMethodInfo->args.retTypeClass, false);
-                    }
-                }
-                else
-                {
-                    returnLocalDsc.lvType = genActualType(comp->info.compRetNativeType);
+                    comp->lvaSetStruct(returnLocalNum, comp->info.compMethodInfo->args.retTypeClass, false);
                 }
             }
             else if (comp->compMethodReturnsRetBufAddr())

src/coreclr/jit/gentree.cpp

@@ -15621,7 +15621,6 @@ GenTree* Compiler::gtNewTempAssign(
         {
             // It could come from `ASG(struct, 0)` that was propagated to `RETURN struct(0)`,
             // and now it is merging to a struct again.
-            assert(!compDoOldStructRetyping());
             assert(tmp == genReturnLocal);
             ok = true;
         }
@@ -15674,7 +15673,6 @@ GenTree* Compiler::gtNewTempAssign(
         // 2. we are propagation `ASG(struct V01, 0)` to `RETURN(struct V01)`, `CNT_INT` doesn't `structHnd`;
         // in these cases, we can use the type of the merge return for the assignment.
         assert(val->OperIs(GT_IND, GT_LCL_FLD, GT_CNS_INT));
-        assert(!compDoOldStructRetyping());
         assert(tmp == genReturnLocal);
         valStructHnd = lvaGetStruct(genReturnLocal);
         assert(valStructHnd != NO_CLASS_HANDLE);
@@ -15682,7 +15680,6 @@ GenTree* Compiler::gtNewTempAssign(
 
     if ((valStructHnd != NO_CLASS_HANDLE) && val->IsConstInitVal())
     {
-        assert(!compDoOldStructRetyping());
         asg = gtNewAssignNode(dest, val);
     }
     else if (varTypeIsStruct(varDsc) && ((valStructHnd != NO_CLASS_HANDLE) || varTypeIsSIMD(valTyp)))

src/coreclr/jit/jitconfigvalues.h

@@ -506,9 +506,6 @@ CONFIG_INTEGER(JitSaveFpLrWithCalleeSavedRegisters, W("JitSaveFpLrWithCalleeSave
 #endif // defined(TARGET_ARM64)
 #endif // DEBUG
 
-CONFIG_INTEGER(JitDoOldStructRetyping, W("JitDoOldStructRetyping"), 0) // Allow Jit to retype structs as primitive types
-                                                                       // when possible.
-
 #undef CONFIG_INTEGER
 #undef CONFIG_STRING
 #undef CONFIG_METHODSET

src/coreclr/jit/lclmorph.cpp

@@ -527,7 +527,7 @@ public:
                     assert(TopValue(0).Node() == node->gtGetOp1());
                     GenTreeUnOp* ret    = node->AsUnOp();
                     GenTree*     retVal = ret->gtGetOp1();
-                    if (!m_compiler->compDoOldStructRetyping() && retVal->OperIs(GT_LCL_VAR))
+                    if (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.

src/coreclr/jit/lclvars.cpp

@@ -2102,8 +2102,7 @@ bool Compiler::StructPromotionHelper::ShouldPromoteStructVar(unsigned lclNum)
             shouldPromote = false;
         }
     }
-    else if (!compiler->compDoOldStructRetyping() && (lclNum == compiler->genReturnLocal) &&
-             (structPromotionInfo.fieldCnt > 1))
+    else if ((lclNum == compiler->genReturnLocal) && (structPromotionInfo.fieldCnt > 1))
     {
         // TODO-1stClassStructs: a temporary solution to keep diffs small, it will be fixed later.
         shouldPromote = false;

src/coreclr/jit/lower.cpp

@@ -243,7 +243,6 @@ GenTree* Lowering::LowerNode(GenTree* node)
         case GT_STORE_OBJ:
             if (node->AsBlk()->Data()->IsCall())
             {
-                assert(!comp->compDoOldStructRetyping());
                 LowerStoreSingleRegCallStruct(node->AsBlk());
                 break;
             }
@@ -2981,33 +2980,15 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
     bool doPrimitiveBitcast = false;
     if (needBitcast)
     {
-        if (comp->compDoOldStructRetyping())
-        {
-            // `struct A { SIMD12/16 }` on `UNIX_AMD64_ABI` is an example when
-            // `varTypeUsesFloatReg` returns different values for `ret` and `ret->gtGetOp1()`,
-            // but doesn't need a primitive bitcase.
-            doPrimitiveBitcast = !ret->TypeIs(TYP_STRUCT);
-        }
-        else
-        {
-            doPrimitiveBitcast = (!varTypeIsStruct(ret) && !varTypeIsStruct(retVal));
-        }
+        doPrimitiveBitcast = (!varTypeIsStruct(ret) && !varTypeIsStruct(retVal));
     }
 
     if (doPrimitiveBitcast)
     {
-// Add a simple bitcast for an old retyping or when both types are not structs.
-// If one type is a struct it will be handled below for !compDoOldStructRetyping.
+// Add a simple bitcast when both types are not structs.
+// If one type is a struct it will be handled below.
 #if defined(DEBUG)
-        if (comp->compDoOldStructRetyping())
-        {
-            assert(varTypeIsSIMD(ret) || !varTypeIsStruct(ret));
-            assert(varTypeIsSIMD(retVal) || !varTypeIsStruct(retVal));
-        }
-        else
-        {
-            assert(!varTypeIsStruct(ret) && !varTypeIsStruct(retVal));
-        }
+        assert(!varTypeIsStruct(ret) && !varTypeIsStruct(retVal));
 #endif
 
         GenTree* bitcast = comp->gtNewBitCastNode(ret->TypeGet(), retVal);
@@ -3033,9 +3014,8 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
 #ifdef DEBUG
         if (varTypeIsStruct(ret->TypeGet()) != varTypeIsStruct(retVal->TypeGet()))
         {
-            if (!comp->compDoOldStructRetyping() && varTypeIsStruct(ret->TypeGet()))
+            if (varTypeIsStruct(ret->TypeGet()))
             {
-                assert(!comp->compDoOldStructRetyping());
                 assert(comp->info.compRetNativeType != TYP_STRUCT);
 
                 var_types retActualType    = genActualType(comp->info.compRetNativeType);
@@ -3059,22 +3039,9 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
         }
         else if (!ret->TypeIs(TYP_VOID) && varTypeIsStruct(retVal))
         {
-            if (comp->compDoOldStructRetyping())
-            {
-#ifdef FEATURE_SIMD
-                assert(ret->TypeIs(TYP_DOUBLE));
-                assert(retVal->TypeIs(TYP_SIMD8));
-#else
-                unreached();
-#endif
-            }
-            else
-            {
-                // Return struct as a primitive using Unsafe cast.
-                assert(!comp->compDoOldStructRetyping());
-                assert(retVal->OperIs(GT_LCL_VAR));
-                LowerRetSingleRegStructLclVar(ret);
-            }
+            // Return struct as a primitive using Unsafe cast.
+            assert(retVal->OperIs(GT_LCL_VAR));
+            LowerRetSingleRegStructLclVar(ret);
         }
     }
 
@@ -3111,7 +3078,6 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
         assert(varDsc->CanBeReplacedWithItsField(comp) || varDsc->lvDoNotEnregister || !varDsc->lvPromoted);
         if (varDsc->CanBeReplacedWithItsField(comp))
         {
-            assert(!comp->compDoOldStructRetyping());
             assert(varDsc->lvFieldCnt == 1);
             unsigned   fldNum = varDsc->lvFieldLclStart;
             LclVarDsc* fldDsc = comp->lvaGetDesc(fldNum);
@@ -3165,7 +3131,6 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
 #if defined(TARGET_XARCH) && !defined(UNIX_AMD64_ABI)
             // Windows x64 doesn't have multireg returns,
             // x86 uses it only for long return type, not for structs.
-            assert(!comp->compDoOldStructRetyping());
             assert(slotCount == 1);
             assert(regType != TYP_UNDEF);
 #else  // !TARGET_XARCH || UNIX_AMD64_ABI
@@ -3177,7 +3142,6 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
                 }
                 else
                 {
-                    assert(!comp->compDoOldStructRetyping());
                     unsigned size = layout->GetSize();
                     assert((size <= 8) || (size == 16));
                     bool isPowerOf2    = (((size - 1) & size) == 0);
@@ -3252,16 +3216,7 @@ void Lowering::LowerRetStruct(GenTreeUnOp* ret)
             {
                 assert(varTypeIsSIMD(ret->gtGetOp1()));
                 assert(comp->compMethodReturnsMultiRegRegTypeAlternate());
-                if (!comp->compDoOldStructRetyping())
-                {
-                    ret->ChangeType(comp->info.compRetNativeType);
-                }
-                else
-                {
-                    // With old struct retyping a value that is returned as HFA
-                    // could have both SIMD* or STRUCT types, keep it as it.
-                    return;
-                }
+                ret->ChangeType(comp->info.compRetNativeType);
             }
             else
             {
@@ -3270,7 +3225,6 @@ void Lowering::LowerRetStruct(GenTreeUnOp* ret)
                 if (retVal->TypeGet() != ret->TypeGet())
                 {
                     assert(retVal->OperIs(GT_LCL_VAR));
-                    assert(!comp->compDoOldStructRetyping());
                     LowerRetSingleRegStructLclVar(ret);
                 }
                 return;
@@ -3284,7 +3238,6 @@ void Lowering::LowerRetStruct(GenTreeUnOp* ret)
         return;
     }
 
-    assert(!comp->compDoOldStructRetyping());
     assert(ret->OperIs(GT_RETURN));
     assert(varTypeIsStruct(ret));
 
@@ -3394,7 +3347,6 @@ void Lowering::LowerRetStruct(GenTreeUnOp* ret)
 void Lowering::LowerRetSingleRegStructLclVar(GenTreeUnOp* ret)
 {
     assert(!comp->compMethodReturnsMultiRegRegTypeAlternate());
-    assert(!comp->compDoOldStructRetyping());
     assert(ret->OperIs(GT_RETURN));
     GenTreeLclVarCommon* lclVar = ret->gtGetOp1()->AsLclVar();
     assert(lclVar->OperIs(GT_LCL_VAR));
@@ -3485,7 +3437,6 @@ void Lowering::LowerCallStruct(GenTreeCall* call)
         }
     }
 
-    assert(!comp->compDoOldStructRetyping());
     CORINFO_CLASS_HANDLE        retClsHnd = call->gtRetClsHnd;
     Compiler::structPassingKind howToReturnStruct;
     var_types returnType = comp->getReturnTypeForStruct(retClsHnd, call->GetUnmanagedCallConv(), &howToReturnStruct);
@@ -3546,8 +3497,6 @@ void Lowering::LowerCallStruct(GenTreeCall* call)
 //
 void Lowering::LowerStoreSingleRegCallStruct(GenTreeBlk* store)
 {
-    assert(!comp->compDoOldStructRetyping());
-    assert(varTypeIsStruct(store));
     assert(store->Data()->IsCall());
     GenTreeCall* call = store->Data()->AsCall();
     assert(!call->HasMultiRegRetVal());

src/coreclr/jit/morph.cpp

@@ -5114,7 +5114,7 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall*         call,
     GenTree* arg     = fgMakeTmpArgNode(argEntry);
 
     // Change the expression to "(tmp=val),tmp"
-    arg = gtNewOperNode(GT_COMMA, arg->TypeGet(), copyBlk, arg);
+    arg                                  = gtNewOperNode(GT_COMMA, arg->TypeGet(), copyBlk, arg);
 
 #endif // FEATURE_FIXED_OUT_ARGS
 
@@ -5166,84 +5166,6 @@ void Compiler::fgAddSkippedRegsInPromotedStructArg(LclVarDsc* varDsc,
 
 #endif // TARGET_ARM
 
-//****************************************************************************
-//  fgFixupStructReturn:
-//    The companion to impFixupCallStructReturn.  Now that the importer is done
-//    change the gtType to the precomputed native return type
-//    requires that callNode currently has a struct type
-//
-void Compiler::fgFixupStructReturn(GenTree* callNode)
-{
-    if (!compDoOldStructRetyping())
-    {
-        return;
-    }
-    assert(varTypeIsStruct(callNode));
-
-    GenTreeCall* call              = callNode->AsCall();
-    bool         callHasRetBuffArg = call->HasRetBufArg();
-    bool         isHelperCall      = call->IsHelperCall();
-
-    // Decide on the proper return type for this call that currently returns a struct
-    //
-    CORINFO_CLASS_HANDLE        retClsHnd = call->gtRetClsHnd;
-    Compiler::structPassingKind howToReturnStruct;
-    var_types                   returnType;
-
-    // There are a couple of Helper Calls that say they return a TYP_STRUCT but they
-    // expect this method to re-type this to a TYP_REF (what is in call->gtReturnType)
-    //
-    //    CORINFO_HELP_METHODDESC_TO_STUBRUNTIMEMETHOD
-    //    CORINFO_HELP_FIELDDESC_TO_STUBRUNTIMEFIELD
-    //    CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE_MAYBENULL
-    //
-    if (isHelperCall)
-    {
-        assert(!callHasRetBuffArg);
-        assert(retClsHnd == NO_CLASS_HANDLE);
-
-        // Now that we are past the importer, re-type this node
-        howToReturnStruct = SPK_PrimitiveType;
-        returnType        = (var_types)call->gtReturnType;
-    }
-    else
-    {
-        returnType = getReturnTypeForStruct(retClsHnd, call->GetUnmanagedCallConv(), &howToReturnStruct);
-    }
-
-    if (howToReturnStruct == SPK_ByReference)
-    {
-        assert(returnType == TYP_UNKNOWN);
-        assert(callHasRetBuffArg);
-    }
-    else
-    {
-        assert(returnType != TYP_UNKNOWN);
-
-        if (!varTypeIsStruct(returnType))
-        {
-            // Widen the primitive type if necessary
-            returnType = genActualType(returnType);
-        }
-        call->gtType = returnType;
-    }
-
-#if FEATURE_MULTIREG_RET
-    // Either we don't have a struct now or if struct, then it is a struct returned in regs or in return buffer.
-    assert((call->gtType != TYP_STRUCT) || call->HasMultiRegRetVal() || callHasRetBuffArg);
-#else // !FEATURE_MULTIREG_RET
-    // No more struct returns
-    assert(call->TypeGet() != TYP_STRUCT);
-#endif
-
-#if !defined(UNIX_AMD64_ABI)
-    // If it was a struct return, it has been transformed into a call
-    // with a return buffer (that returns TYP_VOID) or into a return
-    // of a primitive/enregisterable type
-    assert(!callHasRetBuffArg || (call->TypeGet() == TYP_VOID));
-#endif
-}
-
 /*****************************************************************************
  *
  *  A little helper used to rearrange nested commutative operations. The
@@ -6893,7 +6815,7 @@ bool Compiler::fgCanFastTailCall(GenTreeCall* callee, const char** failReason)
 #ifdef DEBUG
     if (callee->IsTailPrefixedCall())
     {
-        var_types retType = (compDoOldStructRetyping() ? info.compRetNativeType : info.compRetType);
+        var_types retType = info.compRetType;
         assert(impTailCallRetTypeCompatible(retType, info.compMethodInfo->args.retTypeClass, info.compCallConv,
                                             (var_types)callee->gtReturnType, callee->gtRetClsHnd,
                                             callee->GetUnmanagedCallConv()));
@@ -9115,10 +9037,6 @@ Statement* Compiler::fgAssignRecursiveCallArgToCallerParam(GenTree*       arg,
 
 GenTree* Compiler::fgMorphCall(GenTreeCall* call)
 {
-    if (varTypeIsStruct(call))
-    {
-        fgFixupStructReturn(call);
-    }
     if (call->CanTailCall())
     {
         GenTree* newNode = fgMorphPotentialTailCall(call);
@@ -10590,8 +10508,7 @@ GenTree* Compiler::fgMorphGetStructAddr(GenTree** pTree, CORINFO_CLASS_HANDLE cl
             {
                 // TODO: Consider using lvaGrabTemp and gtNewTempAssign instead, since we're
                 // not going to use "temp"
-                GenTree* temp = fgInsertCommaFormTemp(pTree, clsHnd);
-                assert(!compDoOldStructRetyping());
+                GenTree* temp   = fgInsertCommaFormTemp(pTree, clsHnd);
                 unsigned lclNum = temp->gtEffectiveVal()->AsLclVar()->GetLclNum();
                 lvaSetVarDoNotEnregister(lclNum DEBUG_ARG(DNER_VMNeedsStackAddr));
                 addr = fgMorphGetStructAddr(pTree, clsHnd, isRValue);
@@ -10938,7 +10855,7 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree)
     }
 #endif // FEATURE_MULTIREG_RET
 
-    if (src->IsCall() && !compDoOldStructRetyping())
+    if (src->IsCall())
     {
         if (dest->OperIs(GT_OBJ))
         {
@@ -12857,7 +12774,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
 
                 return tree;
             }
-            if (!compDoOldStructRetyping() && !tree->TypeIs(TYP_VOID))
+            if (!tree->TypeIs(TYP_VOID))
             {
                 if (op1->OperIs(GT_OBJ, GT_BLK, GT_IND))
                 {
@@ -15265,7 +15182,6 @@ DONE_MORPHING_CHILDREN:
 //
 GenTree* Compiler::fgMorphRetInd(GenTreeUnOp* ret)
 {
-    assert(!compDoOldStructRetyping());
     assert(ret->OperIs(GT_RETURN));
     assert(ret->gtGetOp1()->OperIs(GT_IND, GT_BLK, GT_OBJ));
     GenTreeIndir* ind  = ret->gtGetOp1()->AsIndir();