From f9fcb8b0643c3ebc81f9e1733097179cebd41561 Mon Sep 17 00:00:00 2001 From: Sergey Andreenko Date: Tue, 13 Apr 2021 00:42:27 -0700 Subject: [PATCH] Delete `JitDoOldStructRetyping` artifacts. (#51092) * Delete `JitDoOldStructRetyping`. * delete unnecessary spilling in `fgUpdateInlineReturnExpressionPlaceHolder`. --- src/coreclr/jit/compiler.h | 6 - src/coreclr/jit/fgbasic.cpp | 9 +- src/coreclr/jit/fginline.cpp | 50 +---- src/coreclr/jit/flowgraph.cpp | 13 +- src/coreclr/jit/gentree.cpp | 3 - src/coreclr/jit/importer.cpp | 338 +++++------------------------- src/coreclr/jit/jitconfigvalues.h | 3 - src/coreclr/jit/lclmorph.cpp | 2 +- src/coreclr/jit/lclvars.cpp | 3 +- src/coreclr/jit/lower.cpp | 69 +----- src/coreclr/jit/morph.cpp | 94 +-------- 11 files changed, 79 insertions(+), 511 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f1e7ffc8672..584e3568f7f 100644 --- a/src/coreclr/jit/compiler.h +++ b/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? diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 2647f6501ef..024c32c2242 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/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 diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index bcb03bc56e5..f31218eaddf 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/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: diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 578967bac52..3210718d041 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/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()) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index a73daa330e5..d4c1a188570 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/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))) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 56a341b3fdd..819e966206d 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -812,11 +812,6 @@ void Compiler::impAssignTempGen(unsigned tmpNum, // type, this would not be necessary - but that requires additional JIT/EE interface // calls that may not actually be required - e.g. if we only access a field of a struct. - if (compDoOldStructRetyping()) - { - val->gtType = varType; - } - GenTree* dst = gtNewLclvNode(tmpNum, varType); asg = impAssignStruct(dst, val, structType, curLevel, pAfterStmt, ilOffset, block); } @@ -1389,12 +1384,6 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, var_types returnType = (var_types)srcCall->gtReturnType; - if (compDoOldStructRetyping()) - { - // We're not using a return buffer, so if we're retyping we'll change the type of 'src' to 'returnTYpe'. - src->gtType = genActualType(returnType); - } - // First we try to change this to "LclVar/LclFld = call" // if ((destAddr->gtOper == GT_ADDR) && (destAddr->AsOp()->gtOp1->gtOper == GT_LCL_VAR)) @@ -1414,14 +1403,6 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, lcl->gtFlags |= GTF_DONT_CSE; varDsc->lvIsMultiRegRet = true; } - else if ((lcl->gtType != src->gtType) && compDoOldStructRetyping()) - { - // We change this to a GT_LCL_FLD (from a GT_ADDR of a GT_LCL_VAR) - lcl->ChangeOper(GT_LCL_FLD); - fgLclFldAssign(lclNum); - lcl->gtType = src->gtType; - asgType = src->gtType; - } dest = lcl; @@ -1474,17 +1455,7 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, { // Case of inline method returning a struct in one or more registers. // We won't need a return buffer - if (compDoOldStructRetyping()) - { - var_types returnType = (var_types)call->gtReturnType; - asgType = returnType; - src->gtType = genActualType(returnType); - call->gtType = src->gtType; - } - else - { - asgType = src->gtType; - } + asgType = src->gtType; if ((destAddr->gtOper != GT_ADDR) || (destAddr->AsOp()->gtOp1->gtOper != GT_LCL_VAR)) { @@ -1624,10 +1595,7 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, dest = gtNewOperNode(GT_IND, asgType, destAddr); } } - else if (compDoOldStructRetyping()) - { - dest->gtType = asgType; - } + if (dest->OperIs(GT_LCL_VAR) && (src->IsMultiRegNode() || (src->OperIs(GT_RET_EXPR) && src->AsRetExpr()->gtInlineCandidate->AsCall()->HasMultiRegRetVal()))) @@ -8494,11 +8462,6 @@ var_types Compiler::impImportCall(OPCODE opcode, CORINFO_CLASS_HANDLE actualMethodRetTypeSigClass; actualMethodRetTypeSigClass = sig->retTypeSigClass; - if (varTypeIsStruct(callRetTyp) && compDoOldStructRetyping()) - { - callRetTyp = impNormStructType(actualMethodRetTypeSigClass); - call->gtType = callRetTyp; - } #if !FEATURE_VARARG /* Check for varargs */ @@ -9475,32 +9438,7 @@ GenTree* Compiler::impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HAN } else if (retRegCount == 1) { - if (!compDoOldStructRetyping()) - { - return call; - } - // See if the struct size is smaller than the return - // type size... - if (retTypeDesc->IsEnclosingType()) - { - // If we know for sure this call will remain a call, - // retype and return value via a suitable temp. - if ((!call->CanTailCall()) && (!call->IsInlineCandidate())) - { - call->gtReturnType = retTypeDesc->GetReturnRegType(0); - return impAssignSmallStructTypeToVar(call, retClsHnd); - } - else - { - call->gtReturnType = call->gtType; - } - } - else - { - // Return type is same size as struct, so we can - // simply retype the call. - call->gtReturnType = retTypeDesc->GetReturnRegType(0); - } + return call; } else { @@ -9538,68 +9476,35 @@ GenTree* Compiler::impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HAN else { -#if FEATURE_MULTIREG_RET +#if !FEATURE_MULTIREG_RET + return call; +#else // FEATURE_MULTIREG_RET const ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc(); const unsigned retRegCount = retTypeDesc->GetReturnRegCount(); assert(retRegCount != 0); - if (!compDoOldStructRetyping() && retRegCount == 1) + if (retRegCount == 1) { return call; } -#else // !FEATURE_MULTIREG_RET - if (!compDoOldStructRetyping()) - { - return call; - } -#endif // !FEATURE_MULTIREG_RET - assert(returnType != TYP_UNKNOWN); - // See if the struct size is smaller than the return - // type size... - if (howToReturnStruct == SPK_EnclosingType) - { - // If we know for sure this call will remain a call, - // retype and return value via a suitable temp. - if ((!call->CanTailCall()) && (!call->IsInlineCandidate())) - { - call->gtReturnType = returnType; - return impAssignSmallStructTypeToVar(call, retClsHnd); - } - } - else - { - // Return type is same size as struct, so we can - // simply retype the call. - call->gtReturnType = returnType; - } + assert(returnType == TYP_STRUCT); + assert((howToReturnStruct == SPK_ByValueAsHfa) || (howToReturnStruct == SPK_ByValue)); - // ToDo: Refactor this common code sequence into its own method as it is used 4+ times - if ((returnType == TYP_LONG) && (compLongUsed == false)) - { - compLongUsed = true; - } - else if (((returnType == TYP_FLOAT) || (returnType == TYP_DOUBLE)) && (compFloatingPointUsed == false)) - { - compFloatingPointUsed = true; - } + assert(call->gtReturnType == returnType); -#if FEATURE_MULTIREG_RET - if (retRegCount >= 2) + assert(retRegCount >= 2); + if ((!call->CanTailCall()) && (!call->IsInlineCandidate())) { - if ((!call->CanTailCall()) && (!call->IsInlineCandidate())) - { - // Force a call returning multi-reg struct to be always of the IR form - // tmp = call - // - // No need to assign a multi-reg struct to a local var if: - // - It is a tail call or - // - The call is marked for in-lining later - return impAssignMultiRegTypeToVar(call, retClsHnd DEBUGARG(call->GetUnmanagedCallConv())); - } + // Force a call returning multi-reg struct to be always of the IR form + // tmp = call + // + // No need to assign a multi-reg struct to a local var if: + // - It is a tail call or + // - The call is marked for in-lining later + return impAssignMultiRegTypeToVar(call, retClsHnd DEBUGARG(call->GetUnmanagedCallConv())); } #endif // FEATURE_MULTIREG_RET } - #endif // not UNIX_AMD64_ABI return call; @@ -9610,6 +9515,18 @@ GenTree* Compiler::impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HAN does not use a struct return buffer */ +//------------------------------------------------------------------------ +// impFixupStructReturnType: For struct return values it sets appropriate flags in MULTIREG returns case; +// in non-multiref case it handles two special helpers: `CORINFO_HELP_GETFIELDSTRUCT`, `CORINFO_HELP_UNBOX_NULLABLE`. +// +// Arguments: +// op - the return value; +// retClsHnd - the struct handle; +// unmgdCallConv - the calling convention of the function that returns this struct. +// +// Return Value: +// the result tree that does the return. +// GenTree* Compiler::impFixupStructReturnType(GenTree* op, CORINFO_CLASS_HANDLE retClsHnd, CorInfoCallConvExtension unmgdCallConv) @@ -9731,95 +9648,31 @@ GenTree* Compiler::impFixupStructReturnType(GenTree* op, #endif // FEATURE_MULTIREG_RET && TARGET_ARM64 - if (!compDoOldStructRetyping() && (!op->IsCall() || !op->AsCall()->TreatAsHasRetBufArg(this))) + if (!op->IsCall() || !op->AsCall()->TreatAsHasRetBufArg(this)) { // Don't retype `struct` as a primitive type in `ret` instruction. return op; } -REDO_RETURN_NODE: - // adjust the type away from struct to integral - // and no normalizing - if (op->gtOper == GT_LCL_VAR) - { - // It is possible that we now have a lclVar of scalar type. - // If so, don't transform it to GT_LCL_FLD. - LclVarDsc* varDsc = lvaGetDesc(op->AsLclVarCommon()); - if (genActualType(varDsc->TypeGet()) != genActualType(info.compRetNativeType)) - { - op->ChangeOper(GT_LCL_FLD); - } - } - else if (op->gtOper == GT_OBJ) - { - GenTree* op1 = op->AsObj()->Addr(); - - // We will fold away OBJ/ADDR, except for OBJ/ADDR/INDEX - // - // In the latter case the OBJ type may have a different type - // than the array element type, and we need to preserve the - // array element type for now. - // - if ((op1->gtOper == GT_ADDR) && (op1->AsOp()->gtOp1->gtOper != GT_INDEX)) - { - // Change '*(&X)' to 'X' and see if we can do better - op = op1->AsOp()->gtOp1; - goto REDO_RETURN_NODE; - } - op->ChangeOperUnchecked(GT_IND); - op->gtFlags |= GTF_IND_TGTANYWHERE; - } - else if (op->gtOper == GT_CALL) - { - if (op->AsCall()->TreatAsHasRetBufArg(this)) - { - // This must be one of those 'special' helpers that don't - // really have a return buffer, but instead use it as a way - // to keep the trees cleaner with fewer address-taken temps. - // - // Well now we have to materialize the the return buffer as - // an address-taken temp. Then we can return the temp. - // - // NOTE: this code assumes that since the call directly - // feeds the return, then the call must be returning the - // same structure/class/type. - // - unsigned tmpNum = lvaGrabTemp(true DEBUGARG("pseudo return buffer")); - - // No need to spill anything as we're about to return. - impAssignTempGen(tmpNum, op, info.compMethodInfo->args.retTypeClass, (unsigned)CHECK_SPILL_NONE); - - if (compDoOldStructRetyping()) - { - // Don't create both a GT_ADDR & GT_OBJ just to undo all of that; instead, - // jump directly to a GT_LCL_FLD. - op = gtNewLclvNode(tmpNum, info.compRetNativeType); - op->ChangeOper(GT_LCL_FLD); - } - else - { - op = gtNewLclvNode(tmpNum, info.compRetType); - JITDUMP("\nimpFixupStructReturnType: created a pseudo-return buffer for a special helper\n"); - DISPTREE(op); - return op; - } - } - else - { - // Don't change the gtType of the call just yet, it will get changed later. - return op; - } - } - else if (op->gtOper == GT_COMMA) - { - op->AsOp()->gtOp2 = impFixupStructReturnType(op->AsOp()->gtOp2, retClsHnd, unmgdCallConv); - } + // This must be one of those 'special' helpers that don't + // really have a return buffer, but instead use it as a way + // to keep the trees cleaner with fewer address-taken temps. + // + // Well now we have to materialize the the return buffer as + // an address-taken temp. Then we can return the temp. + // + // NOTE: this code assumes that since the call directly + // feeds the return, then the call must be returning the + // same structure/class/type. + // + unsigned tmpNum = lvaGrabTemp(true DEBUGARG("pseudo return buffer")); - op->gtType = info.compRetNativeType; + // No need to spill anything as we're about to return. + impAssignTempGen(tmpNum, op, info.compMethodInfo->args.retTypeClass, (unsigned)CHECK_SPILL_NONE); - JITDUMP("\nimpFixupStructReturnType: result of retyping is\n"); + op = gtNewLclvNode(tmpNum, info.compRetType); + JITDUMP("\nimpFixupStructReturnType: created a pseudo-return buffer for a special helper\n"); DISPTREE(op); - return op; } @@ -15738,14 +15591,10 @@ void Compiler::impImportBlockCode(BasicBlock* block) // The handle struct is returned in register op1->AsCall()->gtReturnType = GetRuntimeHandleUnderlyingType(); - if (!compDoOldStructRetyping()) - { - op1->AsCall()->gtRetClsHnd = classHandle; + op1->AsCall()->gtRetClsHnd = classHandle; #if FEATURE_MULTIREG_RET - op1->AsCall()->InitializeStructReturnType(this, classHandle, - op1->AsCall()->GetUnmanagedCallConv()); + op1->AsCall()->InitializeStructReturnType(this, classHandle, op1->AsCall()->GetUnmanagedCallConv()); #endif - } tiRetVal = typeInfo(TI_STRUCT, classHandle); } @@ -15787,13 +15636,10 @@ void Compiler::impImportBlockCode(BasicBlock* block) // The handle struct is returned in register and // it could be consumed both as `TYP_STRUCT` and `TYP_REF`. op1->AsCall()->gtReturnType = GetRuntimeHandleUnderlyingType(); - if (!compDoOldStructRetyping()) - { #if FEATURE_MULTIREG_RET - op1->AsCall()->InitializeStructReturnType(this, tokenType, op1->AsCall()->GetUnmanagedCallConv()); + op1->AsCall()->InitializeStructReturnType(this, tokenType, op1->AsCall()->GetUnmanagedCallConv()); #endif - op1->AsCall()->gtRetClsHnd = tokenType; - } + op1->AsCall()->gtRetClsHnd = tokenType; tiRetVal = verMakeTypeInfo(tokenType); impPushOnStack(op1, tiRetVal); @@ -15970,12 +15816,9 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1 = gtNewHelperCallNode(helper, (var_types)((helper == CORINFO_HELP_UNBOX) ? TYP_BYREF : TYP_STRUCT), gtNewCallArgs(op2, op1)); - if (!compDoOldStructRetyping()) + if (op1->gtType == TYP_STRUCT) { - if (op1->gtType == TYP_STRUCT) - { - op1->AsCall()->gtRetClsHnd = resolvedToken.hClass; - } + op1->AsCall()->gtRetClsHnd = resolvedToken.hClass; } } @@ -17129,68 +16972,11 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) } } - // This is a bit of a workaround... - // If we are inlining a call that returns a struct, where the actual "native" return type is - // not a struct (for example, the struct is composed of exactly one int, and the native - // return type is thus an int), and the inlinee has multiple return blocks (thus, - // fgNeedReturnSpillTemp() == true, and is the index of a local var that is set - // to the *native* return type), and at least one of the return blocks is the result of - // a call, then we have a problem. The situation is like this (from a failed test case): - // - // inliner: - // // Note: valuetype plinq_devtests.LazyTests/LIX is a struct with only a single int - // call !!0 [mscorlib]System.Threading.LazyInitializer::EnsureInitialized(!!0&, bool&, object&, class [mscorlib]System.Func`1) - // - // inlinee: - // ... - // ldobj !!T // this gets bashed to a GT_LCL_FLD, type TYP_INT - // ret - // ... - // call !!0 System.Threading.LazyInitializer::EnsureInitializedCore(!!0&, bool&, - // object&, class System.Func`1) - // ret - // - // In the code above, when we call impFixupStructReturnType(), we will change the op2 return type - // of the inlinee return node, but we don't do that for GT_CALL nodes, which we delay until - // morphing when we call fgFixupStructReturn(). We do this, apparently, to handle nested - // inlining properly by leaving the correct type on the GT_CALL node through importing. - // - // To fix this, for this case, we temporarily change the GT_CALL node type to the - // native return type, which is what it will be set to eventually. We generate the - // assignment to the return temp, using the correct type, and then restore the GT_CALL - // node type. During morphing, the GT_CALL will get the correct, final, native return type. - - bool restoreType = false; - if (compDoOldStructRetyping()) - { - if ((op2->OperGet() == GT_CALL) && (info.compRetType == TYP_STRUCT)) - { - noway_assert(op2->TypeGet() == TYP_STRUCT); - op2->gtType = info.compRetNativeType; - restoreType = true; - } - } - impAssignTempGen(lvaInlineeReturnSpillTemp, op2, se.seTypeInfo.GetClassHandle(), (unsigned)CHECK_SPILL_ALL); - var_types lclRetType = op2->TypeGet(); - if (!compDoOldStructRetyping()) - { - LclVarDsc* varDsc = lvaGetDesc(lvaInlineeReturnSpillTemp); - lclRetType = varDsc->lvType; - } - - GenTree* tmpOp2 = gtNewLclvNode(lvaInlineeReturnSpillTemp, lclRetType); - - if (compDoOldStructRetyping()) - { - if (restoreType) - { - op2->gtType = TYP_STRUCT; // restore it to what it was - } - } + var_types lclRetType = lvaGetDesc(lvaInlineeReturnSpillTemp)->lvType; + GenTree* tmpOp2 = gtNewLclvNode(lvaInlineeReturnSpillTemp, lclRetType); op2 = tmpOp2; #ifdef DEBUG @@ -17459,16 +17245,8 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) #endif op2 = impFixupStructReturnType(op2, retClsHnd, info.compCallConv); // return op2 - var_types returnType; - if (compDoOldStructRetyping()) - { - returnType = info.compRetNativeType; - } - else - { - returnType = info.compRetType; - } - op1 = gtNewOperNode(GT_RETURN, genActualType(returnType), op2); + var_types returnType = info.compRetType; + op1 = gtNewOperNode(GT_RETURN, genActualType(returnType), op2); } else { diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 9191094e094..58156bdb049 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/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 diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 811cfc03806..ce195de08d2 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/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. diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 74b944ebb39..5ed85c89eea 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/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; diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 3eaaac1408b..1c4aac1af08 100644 --- a/src/coreclr/jit/lower.cpp +++ b/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()); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f767015f316..7ce35872d1b 100644 --- a/src/coreclr/jit/morph.cpp +++ b/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(); -- GitLab