From 41882cbf086270042f65445dc54cfec9a4b54c50 Mon Sep 17 00:00:00 2001 From: Sergey Andreenko Date: Wed, 20 May 2020 15:52:13 -0700 Subject: [PATCH] Support !JitDoOldStructRetyping on other platforms. (#35943) * Add more test cases. * Initialize `ReturnTypeDesc` when we keep struct types. * Add a few const modifiers. * Additional checks in `LowerRet` * Support `return double(cnst int)`. * Optimize `LowerRetStruct`: no need for bitcast when read from memory. * Prepare `LowerNode` for store local and local field to multireg. * Compile the new methods with FEATURE_MULTIREG_RET. * Improve `LowerRetStructLclVar`. Don't use bitcast if the source is in memory or has the same type. * Extract `LowerStoreLocCommon`. * Support 3, 5. 6, 7 bytes structs in `LowerCallStruct`. Move call handling to the users. * Disable `JitDoOldStructRetyping` for x86 and x64. Windows x64 was supported in a previous PR, this adds x86 (Windows and Linux) and x64 Unix. * Fix suggestions. * Disable by default for the merge. --- src/coreclr/src/jit/importer.cpp | 32 +- src/coreclr/src/jit/jitconfigvalues.h | 6 +- src/coreclr/src/jit/lower.cpp | 422 +++++++++++++++------ src/coreclr/src/jit/lower.h | 13 +- src/coreclr/src/jit/lowerarmarch.cpp | 4 +- src/coreclr/src/jit/lowerxarch.cpp | 4 +- .../src/JIT/Directed/StructABI/structreturn.cs | 402 ++++++++++++++++++++ 7 files changed, 751 insertions(+), 132 deletions(-) diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 010cd33..9a6f1af 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -9059,10 +9059,21 @@ GenTree* Compiler::impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HAN } else { + +#if FEATURE_MULTIREG_RET + const ReturnTypeDesc* retTypeDesc = call->GetReturnTypeDesc(); + const unsigned retRegCount = retTypeDesc->GetReturnRegCount(); + assert(retRegCount != 0); + if (!compDoOldStructRetyping() && 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 @@ -9095,9 +9106,6 @@ GenTree* Compiler::impFixupCallStructReturn(GenTreeCall* call, CORINFO_CLASS_HAN } #if FEATURE_MULTIREG_RET - const unsigned retRegCount = call->GetReturnTypeDesc()->GetReturnRegCount(); - assert(retRegCount != 0); - if (retRegCount >= 2) { if ((!call->CanTailCall()) && (!call->IsInlineCandidate())) @@ -9130,12 +9138,6 @@ 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); @@ -9250,6 +9252,12 @@ GenTree* Compiler::impFixupStructReturnType(GenTree* op, CORINFO_CLASS_HANDLE re #endif // FEATURE_MULTIREG_RET && FEATURE_HFA + if (!compDoOldStructRetyping() && (!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 @@ -15214,6 +15222,9 @@ void Compiler::impImportBlockCode(BasicBlock* block) if (!compDoOldStructRetyping()) { op1->AsCall()->gtRetClsHnd = classHandle; +#if FEATURE_MULTIREG_RET + op1->AsCall()->InitializeStructReturnType(this, classHandle); +#endif } tiRetVal = typeInfo(TI_STRUCT, classHandle); @@ -15258,6 +15269,9 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1->AsCall()->gtReturnType = GetRuntimeHandleUnderlyingType(); if (!compDoOldStructRetyping()) { +#if FEATURE_MULTIREG_RET + op1->AsCall()->InitializeStructReturnType(this, tokenType); +#endif op1->AsCall()->gtRetClsHnd = tokenType; } diff --git a/src/coreclr/src/jit/jitconfigvalues.h b/src/coreclr/src/jit/jitconfigvalues.h index cb86931..d531362 100644 --- a/src/coreclr/src/jit/jitconfigvalues.h +++ b/src/coreclr/src/jit/jitconfigvalues.h @@ -437,13 +437,13 @@ CONFIG_INTEGER(JitSaveFpLrWithCalleeSavedRegisters, W("JitSaveFpLrWithCalleeSave #endif // defined(TARGET_ARM64) #endif // DEBUG -#if !FEATURE_MULTIREG_RET +#if defined(TARGET_ARMARCH) CONFIG_INTEGER(JitDoOldStructRetyping, W("JitDoOldStructRetyping"), 1) // Allow Jit to retype structs as primitive types // when possible. -#else // FEATURE_MULTIREG_RET +#else CONFIG_INTEGER(JitDoOldStructRetyping, W("JitDoOldStructRetyping"), 1) // Allow Jit to retype structs as primitive types // when possible. -#endif // FEATURE_MULTIREG_RET +#endif #undef CONFIG_INTEGER #undef CONFIG_STRING diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp index 173f470..2c093a6 100644 --- a/src/coreclr/src/jit/lower.cpp +++ b/src/coreclr/src/jit/lower.cpp @@ -37,7 +37,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX // Notes: // If 'childNode' it has any existing sources, they will now be sources for the parent. // -void Lowering::MakeSrcContained(GenTree* parentNode, GenTree* childNode) +void Lowering::MakeSrcContained(GenTree* parentNode, GenTree* childNode) const { assert(!parentNode->OperIsLeaf()); assert(childNode->canBeContained()); @@ -255,6 +255,13 @@ GenTree* Lowering::LowerNode(GenTree* node) case GT_STORE_BLK: case GT_STORE_OBJ: + if (node->AsBlk()->Data()->IsCall()) + { + assert(!comp->compDoOldStructRetyping()); + LowerStoreCallStruct(node->AsBlk()); + break; + } + __fallthrough; case GT_STORE_DYN_BLK: LowerBlockStore(node->AsBlk()); break; @@ -297,64 +304,8 @@ GenTree* Lowering::LowerNode(GenTree* node) __fallthrough; case GT_STORE_LCL_FLD: - { - GenTreeLclVarCommon* const store = node->AsLclVarCommon(); - GenTree* src = store->gtGetOp1(); - if ((varTypeUsesFloatReg(store) != varTypeUsesFloatReg(src)) && !store->IsPhiDefn() && - (src->TypeGet() != TYP_STRUCT)) - { - if (m_lsra->isRegCandidate(comp->lvaGetDesc(store->GetLclNum()))) - { - GenTreeUnOp* bitcast = new (comp, GT_BITCAST) GenTreeOp(GT_BITCAST, store->TypeGet(), src, nullptr); - store->gtOp1 = bitcast; - src = store->gtGetOp1(); - BlockRange().InsertBefore(store, bitcast); - ContainCheckBitCast(bitcast); - } - else - { - // This is an actual store, we'll just retype it. - store->gtType = src->TypeGet(); - } - } - - if ((node->TypeGet() == TYP_STRUCT) && (src->OperGet() != GT_PHI)) - { - LclVarDsc* varDsc = comp->lvaGetDesc(store); -#if FEATURE_MULTIREG_RET - if (src->OperGet() == GT_CALL) - { - assert(src->AsCall()->HasMultiRegRetVal()); - } - else -#endif // FEATURE_MULTIREG_RET - if (!src->OperIs(GT_LCL_VAR, GT_CALL) || varDsc->GetLayout()->GetRegisterType() == TYP_UNDEF) - { - GenTreeLclVar* addr = comp->gtNewLclVarAddrNode(store->GetLclNum(), TYP_BYREF); - - addr->gtFlags |= GTF_VAR_DEF; - assert(!addr->IsPartialLclFld(comp)); - addr->gtFlags |= GTF_DONT_CSE; - - // Create the assignment node. - store->ChangeOper(GT_STORE_OBJ); - - store->gtFlags = GTF_ASG | GTF_IND_NONFAULTING | GTF_IND_TGT_NOT_HEAP; -#ifndef JIT32_GCENCODER - store->AsObj()->gtBlkOpGcUnsafe = false; -#endif - store->AsObj()->gtBlkOpKind = GenTreeObj::BlkOpKindInvalid; - store->AsObj()->SetLayout(varDsc->GetLayout()); - store->AsObj()->SetAddr(addr); - store->AsObj()->SetData(src); - BlockRange().InsertBefore(store, addr); - LowerBlockStore(store->AsObj()); - break; - } - } - LowerStoreLoc(node->AsLclVarCommon()); + LowerStoreLocCommon(node->AsLclVarCommon()); break; - } #if defined(TARGET_ARM64) case GT_CMPXCHG: @@ -1689,13 +1640,10 @@ 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"); @@ -2993,7 +2941,6 @@ void Lowering::LowerRet(GenTreeUnOp* ret) BlockRange().InsertBefore(ret, bitcast); ContainCheckBitCast(bitcast); } -#if !FEATURE_MULTIREG_RET else { #ifdef DEBUG @@ -3002,19 +2949,38 @@ void Lowering::LowerRet(GenTreeUnOp* ret) 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())); + if (varTypeIsStruct(ret->TypeGet())) + { + assert(!comp->compDoOldStructRetyping()); + bool actualTypesMatch = false; + if (genActualType(comp->info.compRetNativeType) == genActualType(retVal->TypeGet())) + { + // This could happen if we have retyped op1 as a primitive type during struct promotion, + // check `retypedFieldsMap` for details. + actualTypesMatch = true; + } + bool constStructInit = retVal->IsConstInitVal(); + assert(actualTypesMatch || constStructInit); + } + else + { +#ifdef FEATURE_SIMD + assert(comp->compDoOldStructRetyping()); + assert(ret->TypeIs(TYP_DOUBLE)); + assert(retVal->TypeIs(TYP_SIMD8)); +#else // !FEATURE_SIMD + unreached(); +#endif // !FEATURE_SIMD + } } } -#endif - if (varTypeIsStruct(ret)) +#endif // DEBUG + if (varTypeIsStruct(ret) && !comp->compMethodReturnsMultiRegRetType()) { 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)) @@ -3024,7 +2990,111 @@ void Lowering::LowerRet(GenTreeUnOp* ret) ContainCheckRet(ret); } -#if !FEATURE_MULTIREG_RET +//---------------------------------------------------------------------------------------------- +// LowerStoreLocCommon: platform idependent part of local var or field store lowering. +// +// Arguments: +// lclStore - The store lcl node to lower. +// +void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore) +{ + assert(lclStore->OperIs(GT_STORE_LCL_FLD, GT_STORE_LCL_VAR)); + GenTree* src = lclStore->gtGetOp1(); + LclVarDsc* varDsc = comp->lvaGetDesc(lclStore); + if ((varTypeUsesFloatReg(lclStore) != varTypeUsesFloatReg(src)) && !lclStore->IsPhiDefn() && + (src->TypeGet() != TYP_STRUCT)) + { + if (m_lsra->isRegCandidate(varDsc)) + { + GenTreeUnOp* bitcast = new (comp, GT_BITCAST) GenTreeOp(GT_BITCAST, lclStore->TypeGet(), src, nullptr); + lclStore->gtOp1 = bitcast; + src = lclStore->gtGetOp1(); + BlockRange().InsertBefore(lclStore, bitcast); + ContainCheckBitCast(bitcast); + } + else + { + // This is an actual store, we'll just retype it. + lclStore->gtType = src->TypeGet(); + } + } + + if ((lclStore->TypeGet() == TYP_STRUCT) && (src->OperGet() != GT_PHI)) + { + if (src->OperGet() == GT_CALL) + { + GenTreeCall* call = src->AsCall(); + const ClassLayout* layout = varDsc->GetLayout(); + const var_types regType = layout->GetRegisterType(); + +#ifdef DEBUG + const unsigned slotCount = layout->GetSlotCount(); +#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 + if (!varDsc->lvIsHfa()) + { + if (slotCount > 1) + { + assert(call->HasMultiRegRetVal()); + } + else + { + assert(!comp->compDoOldStructRetyping()); + unsigned size = layout->GetSize(); + assert((size <= 8) || (size == 16)); + bool isPowerOf2 = (((size - 1) & size) == 0); + bool isTypeDefined = (regType != TYP_UNDEF); + assert(isPowerOf2 == isTypeDefined); + } + } +#endif // !TARGET_XARCH || UNIX_AMD64_ABI +#endif // DEBUG + +#if !defined(WINDOWS_AMD64_ABI) + if (!call->HasMultiRegRetVal() && (regType == TYP_UNDEF)) + { + // If we have a single return register, + // but we can't retype it as a primitive type, we must spill it. + GenTreeLclVar* spilledCall = SpillStructCallResult(call); + lclStore->gtOp1 = spilledCall; + src = lclStore->gtOp1; + LowerStoreLocCommon(lclStore); + return; + } +#endif // !WINDOWS_AMD64_ABI + } + else if (!src->OperIs(GT_LCL_VAR) || varDsc->GetLayout()->GetRegisterType() == TYP_UNDEF) + { + GenTreeLclVar* addr = comp->gtNewLclVarAddrNode(lclStore->GetLclNum(), TYP_BYREF); + + addr->gtFlags |= GTF_VAR_DEF; + assert(!addr->IsPartialLclFld(comp)); + addr->gtFlags |= GTF_DONT_CSE; + + // Create the assignment node. + lclStore->ChangeOper(GT_STORE_OBJ); + GenTreeBlk* objStore = lclStore->AsObj(); + objStore->gtFlags = GTF_ASG | GTF_IND_NONFAULTING | GTF_IND_TGT_NOT_HEAP; +#ifndef JIT32_GCENCODER + objStore->gtBlkOpGcUnsafe = false; +#endif + objStore->gtBlkOpKind = GenTreeObj::BlkOpKindInvalid; + objStore->SetLayout(varDsc->GetLayout()); + objStore->SetAddr(addr); + objStore->SetData(src); + BlockRange().InsertBefore(objStore, addr); + LowerBlockStore(objStore); + return; + } + } + LowerStoreLoc(lclStore); +} + //---------------------------------------------------------------------------------------------- // LowerRetStructLclVar: Lowers a struct return node. // @@ -3033,6 +3103,7 @@ void Lowering::LowerRet(GenTreeUnOp* ret) // void Lowering::LowerRetStruct(GenTreeUnOp* ret) { + assert(!comp->compMethodReturnsMultiRegRetType()); assert(!comp->compDoOldStructRetyping()); assert(ret->OperIs(GT_RETURN)); assert(varTypeIsStruct(ret)); @@ -3050,6 +3121,13 @@ void Lowering::LowerRetStruct(GenTreeUnOp* ret) case GT_CNS_INT: assert(retVal->TypeIs(TYP_INT)); + assert(retVal->AsIntCon()->IconValue() == 0); + if (varTypeUsesFloatReg(nativeReturnType)) + { + retVal->ChangeOperConst(GT_CNS_DBL); + retVal->ChangeType(TYP_FLOAT); + retVal->AsDblCon()->gtDconVal = 0; + } break; case GT_OBJ: @@ -3063,20 +3141,38 @@ void Lowering::LowerRetStruct(GenTreeUnOp* ret) LowerRetStructLclVar(ret); break; +#if defined(FEATURE_SIMD) || defined(FEATURE_HW_INTRINSICS) #ifdef FEATURE_SIMD case GT_SIMD: #endif // FEATURE_SIMD #ifdef FEATURE_HW_INTRINSICS case GT_HWINTRINSIC: #endif // FEATURE_HW_INTRINSICS + { + assert(!retVal->TypeIs(TYP_STRUCT)); + if (varTypeUsesFloatReg(ret) != varTypeUsesFloatReg(retVal)) + { + GenTreeUnOp* bitcast = new (comp, GT_BITCAST) GenTreeOp(GT_BITCAST, ret->TypeGet(), retVal, nullptr); + ret->gtOp1 = bitcast; + BlockRange().InsertBefore(ret, bitcast); + ContainCheckBitCast(bitcast); + } + } + break; +#endif // FEATURE_SIMD || FEATURE_HW_INTRINSICS + 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; +#ifdef DEBUG + GenTreeLclFld* lclFld = retVal->AsLclFld(); + unsigned lclNum = lclFld->GetLclNum(); + LclVarDsc* varDsc = comp->lvaGetDesc(lclNum); + assert(varDsc->lvDoNotEnregister); +#endif + retVal->ChangeType(nativeReturnType); } + break; + default: unreached(); } @@ -3086,15 +3182,21 @@ void Lowering::LowerRetStruct(GenTreeUnOp* ret) // LowerRetStructLclVar: Lowers a return node with a struct lclVar as a source. // // Arguments: -// node - The return node to lower. +// node - The return node to lower. +// +// Notes: +// - if LclVar is allocated in memory then read it as return type; +// - if LclVar can be enregistered read it as register type and add a bitcast if necessary; // void Lowering::LowerRetStructLclVar(GenTreeUnOp* ret) { + assert(!comp->compMethodReturnsMultiRegRetType()); assert(!comp->compDoOldStructRetyping()); assert(ret->OperIs(GT_RETURN)); - GenTreeLclVar* lclVar = ret->gtGetOp1()->AsLclVar(); - unsigned lclNum = lclVar->GetLclNum(); - LclVarDsc* varDsc = comp->lvaGetDesc(lclNum); + GenTreeLclVarCommon* lclVar = ret->gtGetOp1()->AsLclVar(); + assert(lclVar->OperIs(GT_LCL_VAR)); + unsigned lclNum = lclVar->GetLclNum(); + LclVarDsc* varDsc = comp->lvaGetDesc(lclNum); #ifdef DEBUG if (comp->gtGetStructHandleIfPresent(lclVar) == NO_CLASS_HANDLE) @@ -3105,10 +3207,9 @@ void Lowering::LowerRetStructLclVar(GenTreeUnOp* ret) #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. + // We can replace the struct with its only field and keep the field on a register. assert(varDsc->lvRefCnt() == 0); unsigned fieldLclNum = varDsc->lvFieldLclStart; LclVarDsc* fieldDsc = comp->lvaGetDesc(fieldLclNum); @@ -3118,10 +3219,11 @@ void Lowering::LowerRetStructLclVar(GenTreeUnOp* ret) 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; + lclNum = fieldLclNum; + varDsc = comp->lvaGetDesc(lclNum); } } - if (!canEnregister) + else { // TODO-1stClassStructs: We can no longer promote or enregister this struct, // since it is referenced as a whole. @@ -3129,16 +3231,25 @@ void Lowering::LowerRetStructLclVar(GenTreeUnOp* ret) } } - 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)) + if (varDsc->lvDoNotEnregister) { - GenTreeUnOp* bitcast = new (comp, GT_BITCAST) GenTreeOp(GT_BITCAST, ret->TypeGet(), lclVar, nullptr); - ret->gtOp1 = bitcast; - BlockRange().InsertBefore(ret, bitcast); - ContainCheckBitCast(bitcast); + lclVar->ChangeOper(GT_LCL_FLD); + lclVar->AsLclFld()->SetLclOffs(0); + lclVar->ChangeType(ret->TypeGet()); + } + else + { + var_types lclVarType = varDsc->GetRegisterType(lclVar); + assert(lclVarType != TYP_UNDEF); + lclVar->ChangeType(lclVarType); + + if (varTypeUsesFloatReg(ret) != varTypeUsesFloatReg(lclVarType)) + { + GenTreeUnOp* bitcast = new (comp, GT_BITCAST) GenTreeOp(GT_BITCAST, ret->TypeGet(), lclVar, nullptr); + ret->gtOp1 = bitcast; + BlockRange().InsertBefore(ret, bitcast); + ContainCheckBitCast(bitcast); + } } } @@ -3148,16 +3259,32 @@ void Lowering::LowerRetStructLclVar(GenTreeUnOp* ret) // Arguments: // call - The call node to lower. // -// Note: it transforms the call's user. +// Notes: +// - this handles only single-register returns; +// - it transforms the call's user for `GT_STOREIND`. // void Lowering::LowerCallStruct(GenTreeCall* call) { + assert(varTypeIsStruct(call)); + if (call->HasMultiRegRetVal()) + { + return; + } + +#ifdef TARGET_ARMARCH + // !compDoOldStructRetyping is not supported on arm yet, + // because of HFA. + assert(comp->compDoOldStructRetyping()); + return; +#else // !TARGET_ARMARCH + 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. + var_types origType = call->TypeGet(); + call->gtType = genActualType(returnType); LIR::Use callUse; if (BlockRange().TryGetUse(call, &callUse)) @@ -3167,35 +3294,108 @@ void Lowering::LowerCallStruct(GenTreeCall* call) { 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; + // Leave as is, the user will handle it. + assert(user->TypeIs(origType)); + break; case GT_STOREIND: - assert(user->TypeIs(TYP_SIMD8, TYP_REF)); - user->ChangeType(returnType); +#ifdef FEATURE_SIMD + if (user->TypeIs(TYP_SIMD8)) + { + user->ChangeType(returnType); + break; + } +#endif // FEATURE_SIMD + // importer has a separate mechanism to retype calls to helpers, + // keep it for now. + assert(user->TypeIs(TYP_REF)); + assert(call->IsHelperCall()); + assert(returnType == user->TypeGet()); break; default: unreached(); } } +#endif // !TARGET_ARMARCH +} + +//---------------------------------------------------------------------------------------------- +// LowerStoreCallStruct: Lowers a store block where source is a struct typed call. +// +// Arguments: +// store - The store node to lower. +// +// Notes: +// - it spills the call's result if it can be retyped as a primitive type. +// +void Lowering::LowerStoreCallStruct(GenTreeBlk* store) +{ + assert(!comp->compDoOldStructRetyping()); + assert(varTypeIsStruct(store)); + assert(store->Data()->IsCall()); + GenTreeCall* call = store->Data()->AsCall(); + + const ClassLayout* layout = store->GetLayout(); + assert(layout->GetSlotCount() == 1); + const var_types regType = layout->GetRegisterType(); + + unsigned storeSize = store->GetLayout()->GetSize(); + if (regType != TYP_UNDEF) + { + store->ChangeType(regType); + store->SetOper(GT_STOREIND); + LowerStoreIndir(store->AsIndir()); + } + else + { +#if defined(WINDOWS_AMD64_ABI) + // All ABI except Windows x64 supports passing 3 byte structs in registers. + // Other 64 bites ABI-s support passing 5, 6, 7 byte structs. + unreached(); +#else // !WINDOWS_AMD64_ABI + if (store->OperIs(GT_STORE_OBJ)) + { + store->SetOper(GT_STORE_BLK); + } + store->gtBlkOpKind = GenTreeObj::BlkOpKindUnroll; + + GenTreeLclVar* spilledCall = SpillStructCallResult(call); + store->SetData(spilledCall); + LowerBlockStore(store); +#endif // WINDOWS_AMD64_ABI + } +} + +#if !defined(WINDOWS_AMD64_ABI) +//---------------------------------------------------------------------------------------------- +// SpillStructCallResult: Spill call result to memory. +// +// Arguments: +// call - call with 3, 5, 6 or 7 return size that has to be spilled to memory. +// +// Return Value: +// load of the spilled variable. +// +GenTreeLclVar* Lowering::SpillStructCallResult(GenTreeCall* call) const +{ + // TODO-1stClassStructs: we can support this in codegen for `GT_STORE_BLK` without new temps. + const unsigned spillNum = comp->lvaGrabTemp(true DEBUGARG("Return value temp for an odd struct return size")); + CORINFO_CLASS_HANDLE retClsHnd = call->gtRetClsHnd; + comp->lvaSetStruct(spillNum, retClsHnd, false); + GenTreeLclFld* spill = new (comp, GT_STORE_LCL_FLD) GenTreeLclFld(GT_STORE_LCL_FLD, call->gtType, spillNum, 0); + spill->gtOp1 = call; + spill->gtFlags |= GTF_VAR_DEF; + + BlockRange().InsertAfter(call, spill); + ContainCheckStoreLoc(spill); + GenTreeLclVar* loadCallResult = comp->gtNewLclvNode(spillNum, TYP_STRUCT)->AsLclVar(); + BlockRange().InsertAfter(spill, loadCallResult); + return loadCallResult; } -#endif // !FEATURE_MULTIREG_RET +#endif // !WINDOWS_AMD64_ABI GenTree* Lowering::LowerDirectCall(GenTreeCall* call) { diff --git a/src/coreclr/src/jit/lower.h b/src/coreclr/src/jit/lower.h index ae19cd2..b7efee0 100644 --- a/src/coreclr/src/jit/lower.h +++ b/src/coreclr/src/jit/lower.h @@ -92,7 +92,7 @@ private: void ContainCheckStoreIndir(GenTreeIndir* indirNode); void ContainCheckMul(GenTreeOp* node); void ContainCheckShiftRotate(GenTreeOp* node); - void ContainCheckStoreLoc(GenTreeLclVarCommon* storeLoc); + void ContainCheckStoreLoc(GenTreeLclVarCommon* storeLoc) const; void ContainCheckCast(GenTreeCast* node); void ContainCheckCompare(GenTreeOp* node); void ContainCheckBinary(GenTreeOp* node); @@ -132,11 +132,14 @@ private: GenTreeCC* LowerNodeCC(GenTree* node, GenCondition condition); void LowerJmpMethod(GenTree* jmp); void LowerRet(GenTreeUnOp* ret); -#if !FEATURE_MULTIREG_RET + void LowerStoreLocCommon(GenTreeLclVarCommon* lclVar); void LowerRetStruct(GenTreeUnOp* ret); void LowerRetStructLclVar(GenTreeUnOp* ret); void LowerCallStruct(GenTreeCall* call); -#endif + void LowerStoreCallStruct(GenTreeBlk* store); +#if !defined(WINDOWS_AMD64_ABI) + GenTreeLclVar* SpillStructCallResult(GenTreeCall* call) const; +#endif // WINDOWS_AMD64_ABI GenTree* LowerDelegateInvoke(GenTreeCall* call); GenTree* LowerIndirectNonvirtCall(GenTreeCall* call); GenTree* LowerDirectCall(GenTreeCall* call); @@ -459,7 +462,7 @@ public: // return true if 'childNode' is an immediate that can be contained // by the 'parentNode' (i.e. folded into an instruction) // for example small enough and non-relocatable - bool IsContainableImmed(GenTree* parentNode, GenTree* childNode); + bool IsContainableImmed(GenTree* parentNode, GenTree* childNode) const; // Return true if 'node' is a containable memory op. bool IsContainableMemoryOp(GenTree* node) @@ -478,7 +481,7 @@ private: bool AreSourcesPossiblyModifiedLocals(GenTree* addr, GenTree* base, GenTree* index); // Makes 'childNode' contained in the 'parentNode' - void MakeSrcContained(GenTree* parentNode, GenTree* childNode); + void MakeSrcContained(GenTree* parentNode, GenTree* childNode) const; // Checks and makes 'childNode' contained in the 'parentNode' bool CheckImmedAndMakeContained(GenTree* parentNode, GenTree* childNode); diff --git a/src/coreclr/src/jit/lowerarmarch.cpp b/src/coreclr/src/jit/lowerarmarch.cpp index 899531b..ae3d03e 100644 --- a/src/coreclr/src/jit/lowerarmarch.cpp +++ b/src/coreclr/src/jit/lowerarmarch.cpp @@ -52,7 +52,7 @@ bool Lowering::IsCallTargetInRange(void* addr) // TODO-CQ: we can contain a floating point 0.0 constant in a compare instruction // (vcmp on arm, fcmp on arm64). // -bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) +bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const { if (!varTypeIsFloating(parentNode->TypeGet())) { @@ -973,7 +973,7 @@ void Lowering::ContainCheckShiftRotate(GenTreeOp* node) // Arguments: // node - pointer to the node // -void Lowering::ContainCheckStoreLoc(GenTreeLclVarCommon* storeLoc) +void Lowering::ContainCheckStoreLoc(GenTreeLclVarCommon* storeLoc) const { assert(storeLoc->OperIsLocalStore()); GenTree* op1 = storeLoc->gtGetOp1(); diff --git a/src/coreclr/src/jit/lowerxarch.cpp b/src/coreclr/src/jit/lowerxarch.cpp index f2a66d1..4e05fa6 100644 --- a/src/coreclr/src/jit/lowerxarch.cpp +++ b/src/coreclr/src/jit/lowerxarch.cpp @@ -2530,7 +2530,7 @@ bool Lowering::IsCallTargetInRange(void* addr) } // return true if the immediate can be folded into an instruction, for example small enough and non-relocatable -bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) +bool Lowering::IsContainableImmed(GenTree* parentNode, GenTree* childNode) const { if (!childNode->IsIntCnsFitsInI32()) { @@ -3066,7 +3066,7 @@ void Lowering::ContainCheckShiftRotate(GenTreeOp* node) // Arguments: // node - pointer to the node // -void Lowering::ContainCheckStoreLoc(GenTreeLclVarCommon* storeLoc) +void Lowering::ContainCheckStoreLoc(GenTreeLclVarCommon* storeLoc) const { assert(storeLoc->OperIsLocalStore()); GenTree* op1 = storeLoc->gtGetOp1(); diff --git a/src/coreclr/tests/src/JIT/Directed/StructABI/structreturn.cs b/src/coreclr/tests/src/JIT/Directed/StructABI/structreturn.cs index e63b9b2..2268189 100644 --- a/src/coreclr/tests/src/JIT/Directed/StructABI/structreturn.cs +++ b/src/coreclr/tests/src/JIT/Directed/StructABI/structreturn.cs @@ -927,6 +927,406 @@ class TestMergeReturnBlocks } #endregion +class TestHFA +{ + [MethodImpl(MethodImplOptions.NoInlining)] + static float ReturnFloat() + { + return 1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static double ReturnDouble() + { + return 1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Vector2 ReturnVector2() + { + return new Vector2(1); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Vector3 ReturnVector3() + { + return new Vector3(1); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Vector4 ReturnVector4() + { + return new Vector4(1); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Vector4 ReturnVector4UsingCall() + { + return ReturnVector4(); + } + + struct FloatWrapper + { + public float f; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static FloatWrapper ReturnFloatWrapper() + { + return new FloatWrapper(); + } + + struct DoubleWrapper + { + public double f; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static DoubleWrapper ReturnDoubleWrapper() + { + return new DoubleWrapper(); + } + + struct Floats2Wrapper + { + public float f1; + public float f2; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Floats2Wrapper ReturnFloats2Wrapper() + { + return new Floats2Wrapper(); + } + + struct Doubles2Wrapper + { + public double f1; + public double f2; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Doubles2Wrapper ReturnDoubles2Wrapper() + { + return new Doubles2Wrapper(); + } + struct Floats3Wrapper + { + public float f1; + public float f2; + public float f3; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Floats3Wrapper ReturnFloats3Wrapper() + { + return new Floats3Wrapper(); + } + + struct Doubles3Wrapper + { + public double f1; + public double f2; + public double f3; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Doubles3Wrapper ReturnDoubles3Wrapper() + { + return new Doubles3Wrapper(); + } + + struct Floats4Wrapper + { + public float f1; + public float f2; + public float f3; + public float f4; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Floats4Wrapper ReturnFloats4Wrapper() + { + return new Floats4Wrapper(); + } + + struct Doubles4Wrapper + { + public double f1; + public double f2; + public double f3; + public double f4; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Doubles4Wrapper ReturnDoubles4Wrapper() + { + return new Doubles4Wrapper(); + } + + struct Vector2Wrapper + { + Vector2 f1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Vector2Wrapper ReturnVector2Wrapper() + { + return new Vector2Wrapper(); + } + + struct Vector3Wrapper + { + Vector3 f1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Vector3Wrapper ReturnVector3Wrapper() + { + return new Vector3Wrapper(); + } + + struct Vector4Wrapper + { + Vector4 f1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Vector4Wrapper ReturnVector4Wrapper() + { + return new Vector4Wrapper(); + } + + struct Vector2x2Wrapper + { + Vector2 f1; + Vector2 f2; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Vector2x2Wrapper ReturnVector2x2Wrapper() + { + return new Vector2x2Wrapper(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void Test() + { + ReturnFloat(); + ReturnDouble(); + ReturnVector2(); + ReturnVector3(); + ReturnVector4(); + ReturnVector4UsingCall(); + ReturnFloatWrapper(); + ReturnDoubleWrapper(); + ReturnFloats2Wrapper(); + ReturnDoubles2Wrapper(); + ReturnFloats3Wrapper(); + ReturnDoubles3Wrapper(); + ReturnFloats4Wrapper(); + ReturnDoubles4Wrapper(); + ReturnVector2Wrapper(); + ReturnVector3Wrapper(); + ReturnVector4Wrapper(); + ReturnVector2x2Wrapper(); + } +} + +class TestNon2PowerStructs +{ + [StructLayout(LayoutKind.Sequential, Pack = 1)] + public struct Byte3Struct + { + public byte f1; + public byte f2; + public byte f3; + + [MethodImpl(MethodImplOptions.NoInlining)] + public Byte3Struct(int v) + { + f1 = 1; + f2 = 2; + f3 = 3; + } + } + + [StructLayout(LayoutKind.Sequential, Pack = 1)] + public struct Byte5Struct + { + public byte f1; + public byte f2; + public byte f3; + public byte f4; + public byte f5; + + [MethodImpl(MethodImplOptions.NoInlining)] + public Byte5Struct(int v) + { + f1 = 4; + f2 = 5; + f3 = 6; + f4 = 7; + f5 = 8; + } + } + + [StructLayout(LayoutKind.Sequential, Pack = 1)] + public struct Byte6Struct + { + public byte f1; + public byte f2; + public byte f3; + public byte f4; + public byte f5; + public byte f6; + + [MethodImpl(MethodImplOptions.NoInlining)] + public Byte6Struct(int v) + { + f1 = 9; + f2 = 10; + f3 = 11; + f4 = 12; + f5 = 13; + f6 = 14; + } + } + + [StructLayout(LayoutKind.Sequential, Pack = 1)] + public struct Byte7Struct + { + public byte f1; + public byte f2; + public byte f3; + public byte f4; + public byte f5; + public byte f6; + public byte f7; + + [MethodImpl(MethodImplOptions.NoInlining)] + public Byte7Struct(int v) + { + f1 = 15; + f2 = 16; + f3 = 17; + f4 = 18; + f5 = 19; + f6 = 20; + f7 = 21; + } + } + + [StructLayout(LayoutKind.Sequential, Pack = 1)] + struct CompositeOfOddStructs + { + public Byte3Struct a; + public Byte5Struct b; + public Byte6Struct c; + public Byte7Struct d; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Byte3Struct Return3() + { + return new Byte3Struct(0); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Byte5Struct Return5() + { + return new Byte5Struct(0); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Byte6Struct Return6() + { + return new Byte6Struct(0); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Byte7Struct Return7() + { + return new Byte7Struct(0); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static CompositeOfOddStructs CreateComposite() + { + CompositeOfOddStructs c = new CompositeOfOddStructs(); + c.a = Return3(); + c.b = Return5(); + c.c = Return6(); + c.d = Return7(); + return c; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void TestComposite() + { + var c = CreateComposite(); + Debug.Assert(c.a.f1 == 1); + Debug.Assert(c.a.f2 == 2); + Debug.Assert(c.a.f3 == 3); + Debug.Assert(c.b.f1 == 4); + Debug.Assert(c.b.f2 == 5); + Debug.Assert(c.b.f3 == 6); + Debug.Assert(c.b.f4 == 7); + Debug.Assert(c.b.f5 == 8); + Debug.Assert(c.c.f1 == 9); + Debug.Assert(c.c.f2 == 10); + Debug.Assert(c.c.f3 == 11); + Debug.Assert(c.c.f4 == 12); + Debug.Assert(c.c.f5 == 13); + Debug.Assert(c.c.f6 == 14); + Debug.Assert(c.d.f1 == 15); + Debug.Assert(c.d.f2 == 16); + Debug.Assert(c.d.f3 == 17); + Debug.Assert(c.d.f4 == 18); + Debug.Assert(c.d.f5 == 19); + Debug.Assert(c.d.f6 == 20); + Debug.Assert(c.d.f7 == 21); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static byte TestLocals(int v) + { + var a = Return3(); + var a1 = a; + a1.f1 = 0; + var b = Return5(); + var c = Return6(); + var d = Return7(); + if (v == 0) + { + return a.f1; + } + else if (v == 1) + { + return b.f1; + } + else if (v == 3) + { + return c.f1; + } + else if (v == 4) + { + return d.f1; + } + else + { + return a1.f1; + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void Test() + { + TestComposite(); + TestLocals(0); + } +} + class TestStructs { public static int Main() @@ -934,6 +1334,8 @@ class TestStructs TestStructReturns.Test(); TestUnsafeCasts.Test(); TestMergeReturnBlocks.Test(); + TestHFA.Test(); + TestNon2PowerStructs.Test(); return 100; } } -- 2.7.4