From fe9387105408e6fbb13722ee6097fc24b85072f2 Mon Sep 17 00:00:00 2001 From: Sergey Andreenko Date: Wed, 26 Feb 2020 17:03:45 -0800 Subject: [PATCH] Don't retype struct returns x64 windows. --- src/coreclr/src/jit/assertionprop.cpp | 5 + src/coreclr/src/jit/compiler.h | 7 +- src/coreclr/src/jit/compiler.hpp | 6 +- src/coreclr/src/jit/flowgraph.cpp | 22 ++- src/coreclr/src/jit/gentree.cpp | 87 +++++++---- src/coreclr/src/jit/importer.cpp | 107 ++++++++++--- src/coreclr/src/jit/lower.cpp | 208 +++++++++++++++++++++++++- src/coreclr/src/jit/lower.h | 5 + src/coreclr/src/jit/morph.cpp | 119 +++++++++++---- 9 files changed, 481 insertions(+), 85 deletions(-) diff --git a/src/coreclr/src/jit/assertionprop.cpp b/src/coreclr/src/jit/assertionprop.cpp index 69b8f73b07f87..031d4867d87cf 100644 --- a/src/coreclr/src/jit/assertionprop.cpp +++ b/src/coreclr/src/jit/assertionprop.cpp @@ -2679,6 +2679,11 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, newTree->ChangeOperConst(GT_CNS_INT); newTree->AsIntCon()->gtIconVal = curAssertion->op2.u1.iconVal; newTree->ClearIconHandleMask(); + if (newTree->TypeIs(TYP_STRUCT)) + { + // LCL_VAR can be init with a GT_CNS_INT, keep its type INT, not STRUCT. + newTree->ChangeType(TYP_INT); + } } // If we're doing an array index address, assume any constant propagated contributes to the index. if (isArrIndex) diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index a76847e85ba40..c9e5b2af6352f 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -2653,7 +2653,7 @@ class Compiler fgArgTabEntry* gtArgEntryByLateArgIndex(GenTreeCall* call, unsigned lateArgInx); static GenTree* gtArgNodeByLateArgInx(GenTreeCall* call, unsigned lateArgInx); - GenTree* gtNewAssignNode(GenTree* dst, GenTree* src); + GenTreeOp* gtNewAssignNode(GenTree* dst, GenTree* src); GenTree* gtNewTempAssign(unsigned tmp, GenTree* val, @@ -9088,6 +9088,11 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX #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/src/jit/compiler.hpp b/src/coreclr/src/jit/compiler.hpp index 992c7f6c3d120..993c287aea54b 100644 --- a/src/coreclr/src/jit/compiler.hpp +++ b/src/coreclr/src/jit/compiler.hpp @@ -702,6 +702,7 @@ inline bool Compiler::VarTypeIsMultiByteAndCanEnreg( if (varTypeIsStruct(type)) { + assert(typeClass != nullptr); size = info.compCompHnd->getClassSize(typeClass); if (forReturn) { @@ -1807,8 +1808,11 @@ inline void LclVarDsc::incRefCnts(BasicBlock::weight_t weight, Compiler* comp, R // // Increment counts on the local itself. // - if (lvType != TYP_STRUCT || promotionType != Compiler::PROMOTION_TYPE_INDEPENDENT) + if ((lvType != TYP_STRUCT) || (promotionType != Compiler::PROMOTION_TYPE_INDEPENDENT)) { + // We increment ref counts of this local for primitive types, including structs that have been retyped as their + // only field, as well as for structs whose fields are not independently promoted. + // // Increment lvRefCnt // diff --git a/src/coreclr/src/jit/flowgraph.cpp b/src/coreclr/src/jit/flowgraph.cpp index 1a11b13cb4ef2..81f453df18a8f 100644 --- a/src/coreclr/src/jit/flowgraph.cpp +++ b/src/coreclr/src/jit/flowgraph.cpp @@ -6052,7 +6052,14 @@ 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")); - lvaTable[lvaInlineeReturnSpillTemp].lvType = info.compRetNativeType; + if (compDoOldStructRetyping()) + { + lvaTable[lvaInlineeReturnSpillTemp].lvType = info.compRetNativeType; + } + else + { + 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 @@ -8677,7 +8684,18 @@ class MergedReturns if (comp->compMethodReturnsNativeScalarType()) { - returnLocalDsc.lvType = genActualType(comp->info.compRetNativeType); + if (!comp->compDoOldStructRetyping()) + { + 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); + } } else if (comp->compMethodReturnsRetBufAddr()) { diff --git a/src/coreclr/src/jit/gentree.cpp b/src/coreclr/src/jit/gentree.cpp index dfab57adc3583..bc77c5cd4a3a7 100644 --- a/src/coreclr/src/jit/gentree.cpp +++ b/src/coreclr/src/jit/gentree.cpp @@ -6481,7 +6481,7 @@ GenTree* Compiler::gtArgNodeByLateArgInx(GenTreeCall* call, unsigned lateArgInx) * Create a node that will assign 'src' to 'dst'. */ -GenTree* Compiler::gtNewAssignNode(GenTree* dst, GenTree* src) +GenTreeOp* Compiler::gtNewAssignNode(GenTree* dst, GenTree* src) { /* Mark the target as being assigned */ @@ -6498,7 +6498,7 @@ GenTree* Compiler::gtNewAssignNode(GenTree* dst, GenTree* src) /* Create the assignment node */ - GenTree* asg = gtNewOperNode(GT_ASG, dst->TypeGet(), dst, src); + GenTreeOp* asg = gtNewOperNode(GT_ASG, dst->TypeGet(), dst, src)->AsOp(); /* Mark the expression as containing an assignment */ @@ -15137,6 +15137,14 @@ GenTree* Compiler::gtNewTempAssign( // and call returns. Lowering and Codegen will handle these. ok = true; } + else if (dstTyp == TYP_STRUCT && valTyp == TYP_INT) + { + // It could come from `ASG(struct, 0)` that was propogated to `RETURN struct(0)`, + // and now it is merging to a struct again. + assert(!compDoOldStructRetyping()); + assert(tmp == genReturnLocal); + ok = true; + } if (!ok) { @@ -15165,15 +15173,34 @@ GenTree* Compiler::gtNewTempAssign( // struct types. We don't have a convenient way to do that for all SIMD temps, since some // internal trees use SIMD types that are not used by the input IL. In this case, we allow // a null type handle and derive the necessary information about the type from its varType. - CORINFO_CLASS_HANDLE structHnd = gtGetStructHandleIfPresent(val); - if (varTypeIsStruct(varDsc) && ((structHnd != NO_CLASS_HANDLE) || (varTypeIsSIMD(valTyp)))) + CORINFO_CLASS_HANDLE valStructHnd = gtGetStructHandleIfPresent(val); + if (varTypeIsStruct(varDsc) && (valStructHnd == NO_CLASS_HANDLE) && !varTypeIsSIMD(valTyp)) + { + // There are 2 special cases: + // 1. we have lost classHandle from a FIELD node because the parent struct has overlapping fields, + // the field was transformed as IND opr GT_LCL_FLD; + // 2. we are propogating `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); + } + + if ((valStructHnd != NO_CLASS_HANDLE) && val->IsConstInitVal()) + { + assert(!compDoOldStructRetyping()); + asg = gtNewAssignNode(dest, val); + } + else if (varTypeIsStruct(varDsc) && ((valStructHnd != NO_CLASS_HANDLE) || varTypeIsSIMD(valTyp))) { // The struct value may be be a child of a GT_COMMA. GenTree* valx = val->gtEffectiveVal(/*commaOnly*/ true); - if (structHnd != NO_CLASS_HANDLE) + if (valStructHnd != NO_CLASS_HANDLE) { - lvaSetStruct(tmp, structHnd, false); + lvaSetStruct(tmp, valStructHnd, false); } else { @@ -15181,7 +15208,7 @@ GenTree* Compiler::gtNewTempAssign( } dest->gtFlags |= GTF_DONT_CSE; valx->gtFlags |= GTF_DONT_CSE; - asg = impAssignStruct(dest, val, structHnd, (unsigned)CHECK_SPILL_NONE, pAfterStmt, ilOffset, block); + asg = impAssignStruct(dest, val, valStructHnd, (unsigned)CHECK_SPILL_NONE, pAfterStmt, ilOffset, block); } else { @@ -15189,7 +15216,8 @@ GenTree* Compiler::gtNewTempAssign( // when the ABI calls for returning a struct as a primitive type. // TODO-1stClassStructs: When we stop "lying" about the types for ABI purposes, the // 'genReturnLocal' should be the original struct type. - assert(!varTypeIsStruct(valTyp) || typGetObjLayout(structHnd)->GetSize() == genTypeSize(varDsc)); + assert(!varTypeIsStruct(valTyp) || ((valStructHnd != NO_CLASS_HANDLE) && + (typGetObjLayout(valStructHnd)->GetSize() == genTypeSize(varDsc)))); asg = gtNewAssignNode(dest, val); } @@ -17174,6 +17202,10 @@ CORINFO_CLASS_HANDLE Compiler::gtGetStructHandleIfPresent(GenTree* tree) { default: break; + case GT_BITCAST: + assert(!compDoOldStructRetyping()); + structHnd = gtGetStructHandleIfPresent(tree->AsUnOp()->gtOp1); + break; case GT_MKREFANY: structHnd = impGetRefAnyClass(); break; @@ -17239,28 +17271,28 @@ CORINFO_CLASS_HANDLE Compiler::gtGetStructHandleIfPresent(GenTree* tree) } else { - GenTree* addr = tree->AsIndir()->Addr(); + GenTree* addr = tree->AsIndir()->Addr(); + FieldSeqNode* fieldSeq = nullptr; if ((addr->OperGet() == GT_ADD) && addr->gtGetOp2()->OperIs(GT_CNS_INT)) { - FieldSeqNode* fieldSeq = addr->gtGetOp2()->AsIntCon()->gtFieldSeq; - - if (fieldSeq != nullptr) - { - while (fieldSeq->m_next != nullptr) - { - fieldSeq = fieldSeq->m_next; - } - if (fieldSeq != FieldSeqStore::NotAField() && !fieldSeq->IsPseudoField()) - { - CORINFO_FIELD_HANDLE fieldHnd = fieldSeq->m_fieldHnd; - CorInfoType fieldCorType = info.compCompHnd->getFieldType(fieldHnd, &structHnd); - assert(fieldCorType == CORINFO_TYPE_VALUECLASS); - } - } + fieldSeq = addr->gtGetOp2()->AsIntCon()->gtFieldSeq; } - else if (addr->OperGet() == GT_LCL_VAR) + else + { + GetZeroOffsetFieldMap()->Lookup(addr, &fieldSeq); + } + if (fieldSeq != nullptr) { - structHnd = gtGetStructHandleIfPresent(addr); + while (fieldSeq->m_next != nullptr) + { + fieldSeq = fieldSeq->m_next; + } + if (fieldSeq != FieldSeqStore::NotAField() && !fieldSeq->IsPseudoField()) + { + CORINFO_FIELD_HANDLE fieldHnd = fieldSeq->m_fieldHnd; + CorInfoType fieldCorType = info.compCompHnd->getFieldType(fieldHnd, &structHnd); + assert(fieldCorType == CORINFO_TYPE_VALUECLASS); + } } } } @@ -17277,6 +17309,9 @@ CORINFO_CLASS_HANDLE Compiler::gtGetStructHandleIfPresent(GenTree* tree) #endif break; } + // TODO-1stClassStructs: add a check that `structHnd != NO_CLASS_HANDLE`, + // nowadays it won't work because the right part of an ASG could have struct type without a handle + // (check `fgMorphBlockOperand(isBlkReqd`) and a few other cases. } return structHnd; } diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 99d51600991c2..59045c0ddb66e 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -1173,8 +1173,8 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, ilOffset = impCurStmtOffs; } - assert(src->OperIs(GT_LCL_VAR, GT_FIELD, GT_IND, GT_OBJ, GT_CALL, GT_MKREFANY, GT_RET_EXPR, GT_COMMA) || - (src->TypeGet() != TYP_STRUCT && (src->OperIsSimdOrHWintrinsic() || src->OperIs(GT_LCL_FLD)))); + assert(src->OperIs(GT_LCL_VAR, GT_LCL_FLD, GT_FIELD, GT_IND, GT_OBJ, GT_CALL, GT_MKREFANY, GT_RET_EXPR, GT_COMMA) || + (src->TypeGet() != TYP_STRUCT && src->OperIsSimdOrHWintrinsic())); var_types asgType = src->TypeGet(); @@ -1199,8 +1199,11 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, var_types returnType = (var_types)src->AsCall()->gtReturnType; - // We won't use a return buffer, so change the type of src->gtType to 'returnType' - src->gtType = genActualType(returnType); + 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" // @@ -1280,13 +1283,18 @@ GenTree* Compiler::impAssignStructPtr(GenTree* destAddr, else { // Case of inline method returning a struct in one or more registers. - // - var_types returnType = (var_types)call->gtReturnType; - // We won't need a return buffer - asgType = returnType; - src->gtType = genActualType(returnType); - call->gtType = src->gtType; + if (compDoOldStructRetyping()) + { + var_types returnType = (var_types)call->gtReturnType; + asgType = returnType; + src->gtType = genActualType(returnType); + call->gtType = src->gtType; + } + else + { + asgType = src->gtType; + } if ((destAddr->gtOper != GT_ADDR) || (destAddr->AsOp()->gtOp1->gtOper != GT_LCL_VAR)) { @@ -8943,6 +8951,10 @@ GenTree* Compiler::impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HAN } else { + if (!compDoOldStructRetyping()) + { + return call; + } assert(returnType != TYP_UNKNOWN); // See if the struct size is smaller than the return @@ -9010,6 +9022,12 @@ GenTree* Compiler::impFixupStructReturnType(GenTree* op, CORINFO_CLASS_HANDLE re assert(varTypeIsStruct(info.compRetType)); assert(info.compRetBuffArg == BAD_VAR_NUM); + if (!compDoOldStructRetyping() && (!op->IsCall() || !op->AsCall()->TreatAsHasRetBufArg(this))) + { + // Don't retype `struct` as a primitive type in `ret` instruction. + return op; + } + JITDUMP("\nimpFixupStructReturnType: retyping\n"); DISPTREE(op); @@ -9175,10 +9193,20 @@ GenTree* Compiler::impFixupStructReturnType(GenTree* op, CORINFO_CLASS_HANDLE re // No need to spill anything as we're about to return. impAssignTempGen(tmpNum, op, info.compMethodInfo->args.retTypeClass, (unsigned)CHECK_SPILL_NONE); - // 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); + 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 { @@ -15071,10 +15099,16 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1 = gtNewHelperCallNode(CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEHANDLE_MAYBENULL, TYP_STRUCT, helperArgs); + CORINFO_CLASS_HANDLE classHandle = impGetTypeHandleClass(); + // The handle struct is returned in register op1->AsCall()->gtReturnType = GetRuntimeHandleUnderlyingType(); + if (!compDoOldStructRetyping()) + { + op1->AsCall()->gtRetClsHnd = classHandle; + } - tiRetVal = typeInfo(TI_STRUCT, impGetTypeHandleClass()); + tiRetVal = typeInfo(TI_STRUCT, classHandle); } impPushOnStack(op1, tiRetVal); @@ -15111,8 +15145,13 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1 = gtNewHelperCallNode(helper, TYP_STRUCT, helperArgs); - // The handle struct is returned in register + // 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()) + { + op1->AsCall()->gtRetClsHnd = tokenType; + } tiRetVal = verMakeTypeInfo(tokenType); impPushOnStack(op1, tiRetVal); @@ -15289,6 +15328,13 @@ 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) + { + op1->AsCall()->gtRetClsHnd = resolvedToken.hClass; + } + } } assert(helper == CORINFO_HELP_UNBOX && op1->gtType == TYP_BYREF || // Unbox helper returns a byref. @@ -16465,11 +16511,14 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) // node type. During morphing, the GT_CALL will get the correct, final, native return type. bool restoreType = false; - if ((op2->OperGet() == GT_CALL) && (info.compRetType == TYP_STRUCT)) + if (compDoOldStructRetyping()) { - noway_assert(op2->TypeGet() == TYP_STRUCT); - op2->gtType = info.compRetNativeType; - restoreType = true; + 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(), @@ -16477,9 +16526,12 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) GenTree* tmpOp2 = gtNewLclvNode(lvaInlineeReturnSpillTemp, op2->TypeGet()); - if (restoreType) + if (compDoOldStructRetyping()) { - op2->gtType = TYP_STRUCT; // restore it to what it was + if (restoreType) + { + op2->gtType = TYP_STRUCT; // restore it to what it was + } } op2 = tmpOp2; @@ -16706,7 +16758,16 @@ bool Compiler::impReturnInstruction(int prefixFlags, OPCODE& opcode) #endif op2 = impFixupStructReturnType(op2, retClsHnd); // return op2 - op1 = gtNewOperNode(GT_RETURN, genActualType(info.compRetNativeType), op2); + var_types returnType; + if (compDoOldStructRetyping()) + { + returnType = info.compRetNativeType; + } + else + { + returnType = info.compRetType; + } + op1 = gtNewOperNode(GT_RETURN, genActualType(returnType), op2); } else { diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp index 48345ac254cc9..18a05e2a7e57d 100644 --- a/src/coreclr/src/jit/lower.cpp +++ b/src/coreclr/src/jit/lower.cpp @@ -112,8 +112,8 @@ GenTree* Lowering::LowerNode(GenTree* node) switch (node->gtOper) { case GT_IND: - // Leave struct typed indirs alone, they only appear as the source of - // block copy operations and LowerBlockStore will handle those. + // Process struct typed indirs separately, they only appear as the source of + // a block copy operation or a return node. if (node->TypeGet() != TYP_STRUCT) { // TODO-Cleanup: We're passing isContainable = true but ContainCheckIndir rejects @@ -328,7 +328,7 @@ GenTree* Lowering::LowerNode(GenTree* node) } else #endif // FEATURE_MULTIREG_RET - if (!src->OperIs(GT_LCL_VAR) || varDsc->GetLayout()->GetRegisterType() == TYP_UNDEF) + if (!src->OperIs(GT_LCL_VAR, GT_CALL) || varDsc->GetLayout()->GetRegisterType() == TYP_UNDEF) { GenTreeLclVar* addr = comp->gtNewLclVarAddrNode(store->GetLclNum(), TYP_BYREF); @@ -1682,6 +1682,14 @@ void Lowering::LowerCall(GenTree* node) LowerFastTailCall(call); } +#if !FEATURE_MULTIREG_RET + if (varTypeIsStruct(call)) + { + assert(!comp->compDoOldStructRetyping()); + LowerCallStruct(call); + } +#endif // !FEATURE_MULTIREG_RET + ContainCheckCallOperands(call); JITDUMP("lowering call (after):\n"); DISPTREERANGE(BlockRange(), call); @@ -2969,14 +2977,37 @@ void Lowering::LowerRet(GenTreeUnOp* ret) JITDUMP("============"); GenTree* op1 = ret->gtGetOp1(); - if ((ret->TypeGet() != TYP_VOID) && (ret->TypeGet() != TYP_STRUCT) && + if ((ret->TypeGet() != TYP_VOID) && !varTypeIsStruct(ret) && (varTypeUsesFloatReg(ret) != varTypeUsesFloatReg(ret->gtGetOp1()))) { + assert(comp->compDoOldStructRetyping()); GenTreeUnOp* bitcast = new (comp, GT_BITCAST) GenTreeOp(GT_BITCAST, ret->TypeGet(), ret->gtGetOp1(), nullptr); ret->gtOp1 = bitcast; BlockRange().InsertBefore(ret, bitcast); ContainCheckBitCast(bitcast); } +#if !FEATURE_MULTIREG_RET + else + { +#ifdef DEBUG + if (ret->TypeGet() != TYP_VOID) + { + GenTree* retVal = ret->gtGetOp1(); + if (varTypeIsStruct(ret->TypeGet()) != varTypeIsStruct(retVal->TypeGet())) + { + // This could happen if we have retyped op1 as a primitive type during struct promotion, + // check `retypedFieldsMap` for details. + assert(genActualType(comp->info.compRetNativeType) == genActualType(retVal->TypeGet())); + } + } +#endif + if (varTypeIsStruct(ret)) + { + assert(!comp->compDoOldStructRetyping()); + LowerRetStruct(ret); + } + } +#endif // !FEATURE_MULTIREG_RET // Method doing PInvokes has exactly one return block unless it has tail calls. if (comp->compMethodRequiresPInvokeFrame() && (comp->compCurBB == comp->genReturnBB)) @@ -2986,6 +3017,175 @@ void Lowering::LowerRet(GenTreeUnOp* ret) ContainCheckRet(ret); } +#if !FEATURE_MULTIREG_RET +//---------------------------------------------------------------------------------------------- +// LowerRetStructLclVar: Lowers a struct return node. +// +// Arguments: +// node - The return node to lower. +// +void Lowering::LowerRetStruct(GenTreeUnOp* ret) +{ + assert(!comp->compDoOldStructRetyping()); + assert(ret->OperIs(GT_RETURN)); + assert(varTypeIsStruct(ret)); + + GenTree* retVal = ret->gtGetOp1(); + // Note: small types are returned as INT. + var_types nativeReturnType = genActualType(comp->info.compRetNativeType); + ret->ChangeType(nativeReturnType); + + switch (retVal->OperGet()) + { + case GT_CALL: + assert(retVal->TypeIs(nativeReturnType)); // Type should be changed during call processing. + break; + + case GT_CNS_INT: + assert(retVal->TypeIs(TYP_INT)); + break; + + case GT_OBJ: + retVal->ChangeOper(GT_IND); + __fallthrough; + case GT_IND: + retVal->ChangeType(nativeReturnType); + break; + + case GT_LCL_VAR: + LowerRetStructLclVar(ret); + break; + + case GT_SIMD: + case GT_LCL_FLD: + { + GenTreeUnOp* bitcast = new (comp, GT_BITCAST) GenTreeOp(GT_BITCAST, ret->TypeGet(), retVal, nullptr); + ret->gtOp1 = bitcast; + BlockRange().InsertBefore(ret, bitcast); + ContainCheckBitCast(bitcast); + break; + } + default: + unreached(); + } +} + +//---------------------------------------------------------------------------------------------- +// LowerRetStructLclVar: Lowers a return node with a struct lclVar as a source. +// +// Arguments: +// node - The return node to lower. +// +void Lowering::LowerRetStructLclVar(GenTreeUnOp* ret) +{ + assert(!comp->compDoOldStructRetyping()); + assert(ret->OperIs(GT_RETURN)); + GenTreeLclVar* lclVar = ret->gtGetOp1()->AsLclVar(); + unsigned lclNum = lclVar->GetLclNum(); + LclVarDsc* varDsc = comp->lvaGetDesc(lclNum); + +#ifdef DEBUG + if (comp->gtGetStructHandleIfPresent(lclVar) == NO_CLASS_HANDLE) + { + // a promoted struct field was retyped as its only field. + assert(varDsc->lvIsStructField); + } +#endif + if (varDsc->lvPromoted && (comp->lvaGetPromotionType(lclNum) == Compiler::PROMOTION_TYPE_INDEPENDENT)) + { + bool canEnregister = false; + if (varDsc->lvFieldCnt == 1) + { + // We have to replace it with its field. + assert(varDsc->lvRefCnt() == 0); + unsigned fieldLclNum = varDsc->lvFieldLclStart; + LclVarDsc* fieldDsc = comp->lvaGetDesc(fieldLclNum); + if (fieldDsc->lvFldOffset == 0) + { + lclVar->SetLclNum(fieldLclNum); + JITDUMP("Replacing an independently promoted local var with its only field for the return %u, %u\n", + lclNum, fieldLclNum); + lclVar->ChangeType(fieldDsc->lvType); + canEnregister = true; + } + } + if (!canEnregister) + { + // TODO-1stClassStructs: We can no longer promote or enregister this struct, + // since it is referenced as a whole. + comp->lvaSetVarDoNotEnregister(lclNum DEBUGARG(Compiler::DNER_VMNeedsStackAddr)); + } + } + + var_types regType = varDsc->GetRegisterType(lclVar); + assert((regType != TYP_STRUCT) && (regType != TYP_UNKNOWN)); + // Note: regType could be a small type, in this case return will generate an extension move. + lclVar->ChangeType(regType); + if (varTypeUsesFloatReg(ret) != varTypeUsesFloatReg(lclVar)) + { + GenTreeUnOp* bitcast = new (comp, GT_BITCAST) GenTreeOp(GT_BITCAST, ret->TypeGet(), lclVar, nullptr); + ret->gtOp1 = bitcast; + BlockRange().InsertBefore(ret, bitcast); + ContainCheckBitCast(bitcast); + } +} + +//---------------------------------------------------------------------------------------------- +// LowerCallStruct: Lowers a call node that returns a stuct. +// +// Arguments: +// call - The call node to lower. +// +// Note: it transforms the call's user. +// +void Lowering::LowerCallStruct(GenTreeCall* call) +{ + assert(!comp->compDoOldStructRetyping()); + CORINFO_CLASS_HANDLE retClsHnd = call->gtRetClsHnd; + Compiler::structPassingKind howToReturnStruct; + var_types returnType = comp->getReturnTypeForStruct(retClsHnd, &howToReturnStruct); + assert(!varTypeIsStruct(returnType) && returnType != TYP_UNKNOWN); + call->gtType = genActualType(returnType); // the callee normalizes small return types. + + LIR::Use callUse; + if (BlockRange().TryGetUse(call, &callUse)) + { + GenTree* user = callUse.User(); + switch (user->OperGet()) + { + case GT_RETURN: + case GT_STORE_LCL_VAR: + // Leave as is, the user will handle it. + break; + + case GT_STORE_BLK: + case GT_STORE_OBJ: + { + GenTreeBlk* storeBlk = user->AsBlk(); +#ifdef DEBUG + unsigned storeSize = storeBlk->GetLayout()->GetSize(); + assert(storeSize == genTypeSize(returnType)); +#endif // DEBUG + // For `STORE_BLK<2>(dst, call struct<2>)` we will have call retyped as `int`, + // but `STORE` has to use the unwidened type. + user->ChangeType(returnType); + user->SetOper(GT_STOREIND); + } + break; + + case GT_STOREIND: + assert(user->TypeIs(TYP_SIMD8, TYP_REF)); + user->ChangeType(returnType); + user->SetOper(GT_STOREIND); + break; + + default: + unreached(); + } + } +} +#endif // !FEATURE_MULTIREG_RET + GenTree* Lowering::LowerDirectCall(GenTreeCall* call) { noway_assert(call->gtCallType == CT_USER_FUNC || call->gtCallType == CT_HELPER); diff --git a/src/coreclr/src/jit/lower.h b/src/coreclr/src/jit/lower.h index 7608c08f91da5..9c79ede16869a 100644 --- a/src/coreclr/src/jit/lower.h +++ b/src/coreclr/src/jit/lower.h @@ -132,6 +132,11 @@ class Lowering final : public Phase GenTreeCC* LowerNodeCC(GenTree* node, GenCondition condition); void LowerJmpMethod(GenTree* jmp); void LowerRet(GenTreeUnOp* ret); +#if !FEATURE_MULTIREG_RET + void LowerRetStruct(GenTreeUnOp* ret); + void LowerRetStructLclVar(GenTreeUnOp* ret); + void LowerCallStruct(GenTreeCall* call); +#endif GenTree* LowerDelegateInvoke(GenTreeCall* call); GenTree* LowerIndirectNonvirtCall(GenTreeCall* call); GenTree* LowerDirectCall(GenTreeCall* call); diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 69b3eaf80b0df..ab11d827f60d9 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -4980,6 +4980,10 @@ void Compiler::fgAddSkippedRegsInPromotedStructArg(LclVarDsc* varDsc, // void Compiler::fgFixupStructReturn(GenTree* callNode) { + if (!compDoOldStructRetyping()) + { + return; + } assert(varTypeIsStruct(callNode)); GenTreeCall* call = callNode->AsCall(); @@ -6673,7 +6677,8 @@ bool Compiler::fgCanFastTailCall(GenTreeCall* callee, const char** failReason) #ifdef DEBUG if (callee->IsTailPrefixedCall()) { - assert(impTailCallRetTypeCompatible(info.compRetNativeType, info.compMethodInfo->args.retTypeClass, + var_types retType = (compDoOldStructRetyping() ? info.compRetNativeType : info.compRetType); + assert(impTailCallRetTypeCompatible(retType, info.compMethodInfo->args.retTypeClass, (var_types)callee->gtReturnType, callee->gtRetClsHnd)); } #endif @@ -7596,6 +7601,9 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call) { callType = origCallType; } + assert((callType != TYP_UNKNOWN) && !varTypeIsStruct(callType)); + callType = genActualType(callType); + GenTree* zero = gtNewZeroConNode(callType); result = fgMorphTree(zero); } @@ -9313,6 +9321,12 @@ GenTree* Compiler::fgMorphOneAsgBlockOp(GenTree* tree) return nullptr; } + if (src->IsCall() || src->OperIsSIMD()) + { + // Can't take ADDR from these nodes, let fgMorphCopyBlock handle it, #11413. + return nullptr; + } + if ((destVarDsc != nullptr) && !varTypeIsStruct(destVarDsc->TypeGet())) { @@ -9659,12 +9673,6 @@ GenTree* Compiler::fgMorphInitBlock(GenTree* tree) tree->AsOp()->gtOp1 = dest; } tree->gtType = dest->TypeGet(); - // (Constant propagation may cause a TYP_STRUCT lclVar to be changed to GT_CNS_INT, and its - // type will be the type of the original lclVar, in which case we will change it to TYP_INT). - if ((src->OperGet() == GT_CNS_INT) && varTypeIsStruct(src)) - { - src->gtType = TYP_INT; - } JITDUMP("\nfgMorphInitBlock:"); GenTree* oneAsgTree = fgMorphOneAsgBlockOp(tree); @@ -9998,7 +10006,10 @@ 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); - addr = fgMorphGetStructAddr(pTree, clsHnd, isRValue); + assert(!compDoOldStructRetyping()); + unsigned lclNum = temp->gtEffectiveVal()->AsLclVar()->GetLclNum(); + lvaSetVarDoNotEnregister(lclNum DEBUG_ARG(DNER_VMNeedsStackAddr)); + addr = fgMorphGetStructAddr(pTree, clsHnd, isRValue); break; } } @@ -10020,6 +10031,8 @@ GenTree* Compiler::fgMorphGetStructAddr(GenTree** pTree, CORINFO_CLASS_HANDLE cl GenTree* Compiler::fgMorphBlkNode(GenTree* tree, bool isDest) { + JITDUMP("fgMorphBlkNode for %s tree, before:\n", (isDest ? "dst" : "src")); + DISPTREE(tree); GenTree* handleTree = nullptr; GenTree* addr = nullptr; if (tree->OperIs(GT_COMMA)) @@ -10094,6 +10107,8 @@ GenTree* Compiler::fgMorphBlkNode(GenTree* tree, bool isDest) if (!tree->OperIsBlk()) { + JITDUMP("fgMorphBlkNode after:\n"); + DISPTREE(tree); return tree; } GenTreeBlk* blkNode = tree->AsBlk(); @@ -10112,24 +10127,31 @@ GenTree* Compiler::fgMorphBlkNode(GenTree* tree, bool isDest) } else { - return tree; + JITDUMP("fgMorphBlkNode after, DYN_BLK with zero size can't be morphed:\n"); + DISPTREE(blkNode); + return blkNode; } } else { - return tree; + JITDUMP("fgMorphBlkNode after, DYN_BLK with non-const size can't be morphed:\n"); + DISPTREE(blkNode); + return blkNode; } } - if ((blkNode->TypeGet() != TYP_STRUCT) && (blkNode->Addr()->OperGet() == GT_ADDR) && - (blkNode->Addr()->gtGetOp1()->OperGet() == GT_LCL_VAR)) + GenTree* blkSrc = blkNode->Addr(); + assert(blkSrc != nullptr); + if (!blkNode->TypeIs(TYP_STRUCT) && blkSrc->OperIs(GT_ADDR) && blkSrc->gtGetOp1()->OperIs(GT_LCL_VAR)) { - GenTreeLclVarCommon* lclVarNode = blkNode->Addr()->gtGetOp1()->AsLclVarCommon(); + GenTreeLclVarCommon* lclVarNode = blkSrc->gtGetOp1()->AsLclVarCommon(); if ((genTypeSize(blkNode) != genTypeSize(lclVarNode)) || (!isDest && !varTypeIsStruct(lclVarNode))) { lvaSetVarDoNotEnregister(lclVarNode->GetLclNum() DEBUG_ARG(DNER_VMNeedsStackAddr)); } } + JITDUMP("fgMorphBlkNode after:\n"); + DISPTREE(tree); return tree; } @@ -10199,6 +10221,16 @@ GenTree* Compiler::fgMorphBlockOperand(GenTree* tree, var_types asgType, unsigne { lclNode = effectiveVal->AsLclVarCommon(); } + else if (effectiveVal->IsCall()) + { + needsIndirection = false; +#ifdef DEBUG + GenTreeCall* call = effectiveVal->AsCall(); + assert(call->TypeGet() == TYP_STRUCT); + assert(blockWidth == info.compCompHnd->getClassSize(call->gtRetClsHnd)); +#endif + } + if (lclNode != nullptr) { LclVarDsc* varDsc = &(lvaTable[lclNode->GetLclNum()]); @@ -10287,13 +10319,13 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree) { noway_assert(tree->OperIsCopyBlkOp()); - JITDUMP("\nfgMorphCopyBlock:"); + JITDUMP("fgMorphCopyBlock:\n"); bool isLateArg = (tree->gtFlags & GTF_LATE_ARG) != 0; - GenTree* asg = tree; - GenTree* src = asg->gtGetOp2(); - GenTree* dest = asg->gtGetOp1(); + GenTreeOp* asg = tree->AsOp(); + GenTree* src = asg->gtGetOp2(); + GenTree* dest = asg->gtGetOp1(); #if FEATURE_MULTIREG_RET // If this is a multi-reg return, we will not do any morphing of this node. @@ -10310,16 +10342,22 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree) dest = fgMorphBlkNode(dest, true); if (dest != asg->gtGetOp1()) { - asg->AsOp()->gtOp1 = dest; + asg->gtOp1 = dest; if (dest->IsLocal()) { dest->gtFlags |= GTF_VAR_DEF; } } - asg->gtType = dest->TypeGet(); - src = fgMorphBlkNode(src, false); +#ifdef DEBUG + if (asg->TypeGet() != dest->TypeGet()) + { + JITDUMP("changing type of dest from %-6s to %-6s\n", varTypeName(asg->TypeGet()), varTypeName(dest->TypeGet())); + } +#endif + asg->ChangeType(dest->TypeGet()); + src = fgMorphBlkNode(src, false); - asg->AsOp()->gtOp2 = src; + asg->gtOp2 = src; GenTree* oldTree = tree; GenTree* oneAsgTree = fgMorphOneAsgBlockOp(tree); @@ -11071,15 +11109,13 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree) { tree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; } - - if (verbose) - { - printf("\nfgMorphCopyBlock (after):\n"); - gtDispTree(tree); - } #endif _Done: + + JITDUMP("\nfgMorphCopyBlock (after):\n"); + DISPTREE(tree); + return tree; } @@ -11876,6 +11912,31 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) return tree; } + if (tree->TypeIs(TYP_STRUCT) && op1->OperIs(GT_OBJ, GT_BLK)) + { + assert(!compDoOldStructRetyping()); + GenTree* addr = op1->AsBlk()->Addr(); + // if we return `OBJ` or `BLK` from a local var, lcl var has to have a stack address. + if (addr->OperIs(GT_ADDR) && addr->gtGetOp1()->OperIs(GT_LCL_VAR)) + { + GenTreeLclVar* lclVar = addr->gtGetOp1()->AsLclVar(); + assert(!gtIsActiveCSE_Candidate(addr) && !gtIsActiveCSE_Candidate(op1)); + if (gtGetStructHandle(tree) == gtGetStructHandleIfPresent(lclVar)) + { + // Fold *(&x). + tree->AsUnOp()->gtOp1 = op1; + DEBUG_DESTROY_NODE(op1); + DEBUG_DESTROY_NODE(addr); + op1 = lclVar; + } + else + { + // TODO-1stClassStructs: It is not address-taken or block operation, + // but the current IR doesn't allow to express that cast without stack, see #11413. + lvaSetVarDoNotEnregister(lclVar->GetLclNum() DEBUGARG(DNER_BlockOp)); + } + } + } break; case GT_EQ: @@ -13313,7 +13374,9 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) // is a local or clsVar, even if it has been address-exposed. if (op1->OperGet() == GT_ADDR) { - tree->gtFlags |= (op1->gtGetOp1()->gtFlags & GTF_GLOB_REF); + GenTreeUnOp* addr = op1->AsUnOp(); + GenTree* addrOp = addr->gtGetOp1(); + tree->gtFlags |= (addrOp->gtFlags & GTF_GLOB_REF); } break;