diff --git a/src/coreclr/jit/copyprop.cpp b/src/coreclr/jit/copyprop.cpp index 778dd7d404f6eb..fef2e363cbb81f 100644 --- a/src/coreclr/jit/copyprop.cpp +++ b/src/coreclr/jit/copyprop.cpp @@ -257,7 +257,6 @@ void Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned // // Returns: // - lclNum if the local is participating in SSA; -// - fieldLclNum if the parent local can be replaced by its only field; // - BAD_VAR_NUM otherwise. // unsigned Compiler::optIsSsaLocal(GenTreeLclVarCommon* lclNode) @@ -265,11 +264,6 @@ unsigned Compiler::optIsSsaLocal(GenTreeLclVarCommon* lclNode) unsigned lclNum = lclNode->GetLclNum(); LclVarDsc* varDsc = lvaGetDesc(lclNum); - if (!lvaInSsa(lclNum) && varDsc->CanBeReplacedWithItsField(this)) - { - lclNum = varDsc->lvFieldLclStart; - } - if (!lvaInSsa(lclNum)) { return BAD_VAR_NUM; diff --git a/src/coreclr/jit/earlyprop.cpp b/src/coreclr/jit/earlyprop.cpp index 3adaf9e5fd0f25..3a6b744b7ee3b4 100644 --- a/src/coreclr/jit/earlyprop.cpp +++ b/src/coreclr/jit/earlyprop.cpp @@ -374,8 +374,7 @@ GenTree* Compiler::optPropGetValueRec(unsigned lclNum, unsigned ssaNum, optPropK GenTree* treeRhs = ssaDefAsg->gtGetOp2(); - if (treeRhs->OperIsScalarLocal() && lvaInSsa(treeRhs->AsLclVarCommon()->GetLclNum()) && - treeRhs->AsLclVarCommon()->HasSsaName()) + if (treeRhs->OperIsScalarLocal() && treeRhs->AsLclVarCommon()->HasSsaName()) { // Recursively track the Rhs unsigned rhsLclNum = treeRhs->AsLclVarCommon()->GetLclNum(); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index b15378b8e9b979..4d27b8d4becc07 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -23307,6 +23307,30 @@ unsigned GenTreeHWIntrinsic::GetResultOpNumForFMA(GenTree* use, GenTree* op1, Ge } #endif // TARGET_XARCH && FEATURE_HW_INTRINSICS +//------------------------------------------------------------------------ +// HasSsaName: Does this local node have an SSA name? +// +// Return Value: +// Whether this node's SSA name is not RESERVED_SSA_NUM. +// +// Notes: +// If the node has an SSA name, it will always represent a local that +// participates in SSA. Thus, "HasSsaName" implies "lvaIsInSsa". The +// opposite is not true - a node may be in an unreachable block not +// visited by the renamer. +// +bool GenTreeLclVarCommon::HasSsaName() const +{ + bool hasSsaName = GetSsaNum() != SsaConfig::RESERVED_SSA_NUM; + + if (hasSsaName) + { + assert(JitTls::GetCompiler()->lvaInSsa(GetLclNum())); + } + + return hasSsaName; +} + unsigned GenTreeLclFld::GetSize() const { return TypeIs(TYP_STRUCT) ? GetLayout()->GetSize() : genTypeSize(TypeGet()); diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index f81aaa779b8195..4929503e72226e 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -3550,10 +3550,7 @@ struct GenTreeLclVarCommon : public GenTreeUnOp _gtSsaNum = ssaNum; } - bool HasSsaName() - { - return (GetSsaNum() != SsaConfig::RESERVED_SSA_NUM); - } + bool HasSsaName() const; #if DEBUGGABLE_GENTREE GenTreeLclVarCommon() : GenTreeUnOp() diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 84bb325bb1e830..7bedd6eee41d11 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -246,6 +246,9 @@ GenTree* Lowering::LowerNode(GenTree* node) LowerRet(node->AsUnOp()); break; + case GT_BITCAST: + return LowerBitcast(node->AsUnOp()); + case GT_RETURNTRAP: ContainCheckReturnTrap(node->AsOp()); break; @@ -3360,6 +3363,44 @@ void Lowering::LowerRet(GenTreeUnOp* ret) ContainCheckRet(ret); } +//------------------------------------------------------------------------ +// LowerBitcast: Lower a GT_BITCAST node. +// +// Will remove bitcasts that do not move between different register files. +// Such bitcasts are generated in block morphing for assignments of struct +// locals that "can be replaced with their one field" to calls. This makes +// the frontend (SSA) simpler, but here, in the backend, they end up as +// unnecessary no-op BITCAST(type -> type) nodes, so we remove them. +// +// Arguments: +// bitcast - the bitcast node to lower +// +// Return Value: +// The next node to lower. +// +GenTree* Lowering::LowerBitcast(GenTreeUnOp* bitcast) +{ + GenTree* nextNode = bitcast->gtNext; + GenTree* src = bitcast->gtGetOp1(); + + if (bitcast->TypeGet() == src->TypeGet()) + { + LIR::Use bitcastUse; + if (BlockRange().TryGetUse(bitcast, &bitcastUse)) + { + bitcastUse.ReplaceWith(src); + } + else + { + src->SetUnusedValue(); + } + + BlockRange().Remove(bitcast); + } + + return nextNode; +} + //---------------------------------------------------------------------------------------------- // LowerStoreLocCommon: platform idependent part of local var or field store lowering. // @@ -3814,6 +3855,10 @@ void Lowering::LowerCallStruct(GenTreeCall* call) GenTree* user = callUse.User(); switch (user->OperGet()) { + case GT_BITCAST: + assert(genTypeSize(user) == genTypeSize(call)); + break; + case GT_RETURN: case GT_STORE_LCL_VAR: case GT_STORE_BLK: diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 46e1accf726cb3..f3b4ad8d7e450c 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -136,6 +136,7 @@ class Lowering final : public Phase GenTreeCC* LowerNodeCC(GenTree* node, GenCondition condition); void LowerJmpMethod(GenTree* jmp); void LowerRet(GenTreeUnOp* ret); + GenTree* LowerBitcast(GenTreeUnOp* node); void LowerStoreLocCommon(GenTreeLclVarCommon* lclVar); void LowerRetStruct(GenTreeUnOp* ret); void LowerRetSingleRegStructLclVar(GenTreeUnOp* ret); diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index bb02dcc9de9d3e..720866c4ad6aab 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -3523,11 +3523,11 @@ int LinearScan::BuildStoreLoc(GenTreeLclVarCommon* storeLoc) else if (op1->isContained() && op1->OperIs(GT_BITCAST)) { GenTree* bitCastSrc = op1->gtGetOp1(); - RegisterType registerType = bitCastSrc->TypeGet(); + RegisterType registerType = regType(getDefType(bitCastSrc)); singleUseRef = BuildUse(bitCastSrc, allRegs(registerType)); Interval* srcInterval = singleUseRef->getInterval(); - assert(srcInterval->registerType == registerType); + assert(srcInterval->registerType == getDefType(bitCastSrc)); srcCount = 1; } #ifndef TARGET_64BIT diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp index 0346eeb2456ecd..9c881377a323f1 100644 --- a/src/coreclr/jit/morphblock.cpp +++ b/src/coreclr/jit/morphblock.cpp @@ -54,6 +54,7 @@ class MorphInitBlockHelper OneAsgBlock, StructBlock, SkipCallSrc, + BitcastCallSrc, SkipMultiRegIntrinsicSrc, Nop }; @@ -806,15 +807,33 @@ void MorphCopyBlockHelper::TrySpecialCases() m_asg->gtOp1 = lclVar; } } + if (m_dst->OperIs(GT_LCL_VAR)) { LclVarDsc* varDsc = m_comp->lvaGetDesc(m_dst->AsLclVar()); if (varTypeIsStruct(varDsc) && varDsc->CanBeReplacedWithItsField(m_comp)) { + assert(m_comp->fgGlobalMorph); + + JITDUMP("Morphing a single reg call return into BITCAST assigned to the promoted field\n"); + m_transformationDecision = BlockTransformation::BitcastCallSrc; + + unsigned fieldNum = varDsc->lvFieldLclStart; + LclVarDsc* fieldDsc = m_comp->lvaGetDesc(fieldNum); + var_types fieldType = fieldDsc->TypeGet(); + assert(!varTypeIsStruct(fieldType)); + m_dst->gtFlags |= GTF_DONT_CSE; - JITDUMP("Not morphing a single reg call return\n"); - m_transformationDecision = BlockTransformation::SkipCallSrc; - m_result = m_asg; + m_dst->ChangeType(fieldType); + m_dst->AsLclVar()->SetLclNum(fieldNum); + + assert(fieldDsc->lvNormalizeOnLoad() || !varTypeIsSmall(fieldDsc)); + GenTree* bitcast = m_comp->gtNewOperNode(GT_BITCAST, genActualType(fieldType), m_src); + m_src = bitcast; + m_asg->gtOp2 = bitcast; + + m_asg->ChangeType(fieldType); + m_result = m_asg; } } } diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 55cebdb5597bc0..eae3794031ee43 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -7264,7 +7264,7 @@ void Compiler::optHoistLoopBlocks(unsigned loopNum, ArrayStack* blo // To be invariant a LclVar node must not be the LHS of an assignment ... bool isInvariant = !user->OperIs(GT_ASG) || (user->AsOp()->gtGetOp1() != tree); // and the variable must be in SSA ... - isInvariant = isInvariant && m_compiler->lvaInSsa(lclNum) && lclVar->HasSsaName(); + isInvariant = isInvariant && lclVar->HasSsaName(); // and the SSA definition must be outside the loop we're hoisting from ... isInvariant = isInvariant && !m_compiler->optLoopTable[m_loopNum].lpContains( @@ -8455,7 +8455,7 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) { // If it's a local byref for which we recorded a value number, use that... GenTreeLclVar* argLcl = arg->AsLclVar(); - if (lvaInSsa(argLcl->GetLclNum()) && argLcl->HasSsaName()) + if (argLcl->HasSsaName()) { ValueNum argVN = lvaTable[argLcl->GetLclNum()].GetPerSsaData(argLcl->GetSsaNum())->m_vnPair.GetLiberal(); @@ -8533,7 +8533,7 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk) if (rhsVN != ValueNumStore::NoVN) { rhsVN = vnStore->VNNormalValue(rhsVN); - if (lvaInSsa(lhsLcl->GetLclNum()) && lhsLcl->HasSsaName()) + if (lhsLcl->HasSsaName()) { lvaTable[lhsLcl->GetLclNum()] .GetPerSsaData(lhsLcl->GetSsaNum()) diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index b94f124d003280..3cf6e4c97f74fa 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -454,11 +454,7 @@ LclSsaVarDsc* RangeCheck::GetSsaDefAsg(GenTreeLclVarCommon* lclUse) return nullptr; } - LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclUse); - if (varDsc->CanBeReplacedWithItsField(m_pCompiler)) - { - varDsc = m_pCompiler->lvaGetDesc(varDsc->lvFieldLclStart); - } + LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclUse); LclSsaVarDsc* ssaDef = varDsc->GetPerSsaData(ssaNum); // RangeCheck does not care about uninitialized variables. @@ -493,10 +489,6 @@ LclSsaVarDsc* RangeCheck::GetSsaDefAsg(GenTreeLclVarCommon* lclUse) UINT64 RangeCheck::HashCode(unsigned lclNum, unsigned ssaNum) { LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclNum); - if (varDsc->CanBeReplacedWithItsField(m_pCompiler)) - { - lclNum = varDsc->lvFieldLclStart; - } assert(ssaNum != SsaConfig::RESERVED_SSA_NUM); return UINT64(lclNum) << 32 | ssaNum; } @@ -563,11 +555,7 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP return; } - LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lcl); - if (varDsc->CanBeReplacedWithItsField(m_pCompiler)) - { - varDsc = m_pCompiler->lvaGetDesc(varDsc->lvFieldLclStart); - } + LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lcl); LclSsaVarDsc* ssaData = varDsc->GetPerSsaData(lcl->GetSsaNum()); ValueNum normalLclVN = m_pCompiler->vnStore->VNConservativeNormalValue(ssaData->m_vnPair); MergeEdgeAssertions(normalLclVN, assertions, pRange); diff --git a/src/coreclr/jit/ssabuilder.cpp b/src/coreclr/jit/ssabuilder.cpp index 7f62e8c38c7627..da1080ee1017bb 100644 --- a/src/coreclr/jit/ssabuilder.cpp +++ b/src/coreclr/jit/ssabuilder.cpp @@ -745,13 +745,6 @@ void SsaBuilder::RenameDef(GenTree* defNode, BasicBlock* block) unsigned lclNum = lclNode->GetLclNum(); LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclNum); - if (!m_pCompiler->lvaInSsa(lclNum) && varDsc->CanBeReplacedWithItsField(m_pCompiler)) - { - lclNum = varDsc->lvFieldLclStart; - varDsc = m_pCompiler->lvaGetDesc(lclNum); - assert(isFullDef); - } - if (m_pCompiler->lvaInSsa(lclNum)) { // Promoted variables are not in SSA, only their fields are. diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index b203ea7b107849..dd02a74c802074 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8188,24 +8188,6 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree) fgValueNumberLocalStore(tree, lclVarTree, offset, storeSize, rhsVNPair); } - else if (lclVarTree->HasSsaName()) - { - // The local wasn't in SSA, the tree is still an SSA def. There is only one - // case when this can happen - a promoted "CanBeReplacedWithItsField" struct. - assert((lhs == lclVarTree) && rhs->IsCall() && isEntire); - assert(lhsVarDsc->CanBeReplacedWithItsField(this)); - // Give a new, unique, VN to the field. - LclVarDsc* fieldVarDsc = lvaGetDesc(lhsVarDsc->lvFieldLclStart); - LclSsaVarDsc* fieldVarSsaDsc = fieldVarDsc->GetPerSsaData(lclVarTree->GetSsaNum()); - ValueNum newUniqueVN = vnStore->VNForExpr(compCurBB, fieldVarDsc->TypeGet()); - - fieldVarSsaDsc->m_vnPair.SetBoth(newUniqueVN); - - JITDUMP("Tree [%06u] assigned VN to the only field V%02u/%u of promoted struct V%02u: new uniq ", - dspTreeID(tree), lhsVarDsc->lvFieldLclStart, lclVarTree->GetSsaNum(), lhsLclNum); - JITDUMPEXEC(vnPrint(newUniqueVN, 1)); - JITDUMP("\n"); - } else if (lhsVarDsc->IsAddressExposed()) { fgMutateAddressExposedLocal(tree DEBUGARG("INITBLK/COPYBLK - address-exposed local")); @@ -8324,7 +8306,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) { unsigned lclNum = lclFld->GetLclNum(); - if (!lvaInSsa(lclFld->GetLclNum()) || !lclFld->HasSsaName()) + if (!lclFld->HasSsaName()) { lclFld->gtVNPair.SetBoth(vnStore->VNForExpr(compCurBB, lclFld->TypeGet())); }