From 7546409acfc04d32d52c17e5d2c588c71d096cc7 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 8 Jul 2024 15:41:22 +0200 Subject: [PATCH 1/2] JIT: Handle retbuf definitions precise in physical promotion's liveness Fix #86711 --- src/coreclr/jit/promotion.h | 17 +++--- src/coreclr/jit/promotionliveness.cpp | 81 +++++++++++++-------------- 2 files changed, 49 insertions(+), 49 deletions(-) diff --git a/src/coreclr/jit/promotion.h b/src/coreclr/jit/promotion.h index ba5f6fbfa9c67..cc39b9d3ada85 100644 --- a/src/coreclr/jit/promotion.h +++ b/src/coreclr/jit/promotion.h @@ -219,14 +219,15 @@ class PromotionLiveness StructDeaths GetDeathsForStructLocal(GenTreeLclVarCommon* use); private: - void MarkUseDef(GenTreeLclVarCommon* lcl, BitVec& useSet, BitVec& defSet); - void MarkIndex(unsigned index, bool isUse, bool isDef, BitVec& useSet, BitVec& defSet); - void ComputeUseDefSets(); - void InterBlockLiveness(); - bool PerBlockLiveness(BasicBlock* block); - void AddHandlerLiveVars(BasicBlock* block, BitVec& ehLiveVars); - void FillInLiveness(); - void FillInLiveness(BitVec& life, BitVec volatileVars, GenTreeLclVarCommon* lcl); + void MarkUseDef(Statement* stmt, GenTreeLclVarCommon* lcl, BitVec& useSet, BitVec& defSet); + unsigned GetSizeOfStructLocal(Statement* stmt, GenTreeLclVarCommon* lcl); + void MarkIndex(unsigned index, bool isUse, bool isDef, BitVec& useSet, BitVec& defSet); + void ComputeUseDefSets(); + void InterBlockLiveness(); + bool PerBlockLiveness(BasicBlock* block); + void AddHandlerLiveVars(BasicBlock* block, BitVec& ehLiveVars); + void FillInLiveness(); + void FillInLiveness(BitVec& life, BitVec volatileVars, Statement* stmt, GenTreeLclVarCommon* lcl); #ifdef DEBUG void DumpVarSet(BitVec set, BitVec allVars); #endif diff --git a/src/coreclr/jit/promotionliveness.cpp b/src/coreclr/jit/promotionliveness.cpp index 174388ecb129f..b482083fbbe24 100644 --- a/src/coreclr/jit/promotionliveness.cpp +++ b/src/coreclr/jit/promotionliveness.cpp @@ -133,7 +133,7 @@ void PromotionLiveness::ComputeUseDefSets() { for (GenTreeLclVarCommon* lcl : stmt->LocalsTreeList()) { - MarkUseDef(lcl, bb.VarUse, bb.VarDef); + MarkUseDef(stmt, lcl, bb.VarUse, bb.VarDef); } } else @@ -143,7 +143,7 @@ void PromotionLiveness::ComputeUseDefSets() // Skip liveness updates/marking for defs; they may be conditionally executed. if ((lcl->gtFlags & GTF_VAR_DEF) == 0) { - MarkUseDef(lcl, bb.VarUse, bb.VarDef); + MarkUseDef(stmt, lcl, bb.VarUse, bb.VarDef); } } } @@ -155,7 +155,7 @@ void PromotionLiveness::ComputeUseDefSets() { for (GenTreeLclVarCommon* lcl : stmt->LocalsTreeList()) { - MarkUseDef(lcl, bb.VarUse, bb.VarDef); + MarkUseDef(stmt, lcl, bb.VarUse, bb.VarDef); } } } @@ -179,11 +179,12 @@ void PromotionLiveness::ComputeUseDefSets() // Mark use/def information for a single appearence of a local. // // Parameters: +// stmt - Statement containing the local // lcl - The local node // useSet - The use set to mark in. // defSet - The def set to mark in. // -void PromotionLiveness::MarkUseDef(GenTreeLclVarCommon* lcl, BitVec& useSet, BitVec& defSet) +void PromotionLiveness::MarkUseDef(Statement* stmt, GenTreeLclVarCommon* lcl, BitVec& useSet, BitVec& defSet) { AggregateInfo* agg = m_aggregates.Lookup(lcl->GetLclNum()); if (agg == nullptr) @@ -198,17 +199,8 @@ void PromotionLiveness::MarkUseDef(GenTreeLclVarCommon* lcl, BitVec& useSet, Bit unsigned baseIndex = m_structLclToTrackedIndex[lcl->GetLclNum()]; var_types accessType = lcl->TypeGet(); - if (accessType == TYP_STRUCT) + if ((accessType == TYP_STRUCT) || lcl->OperIs(GT_LCL_ADDR)) { - if (lcl->OperIs(GT_LCL_ADDR)) - { - // For LCL_ADDR this is a retbuf and we expect it to be a def. We - // don't know the exact size here so we cannot mark anything as - // being fully defined, thus we can just return. - assert(isDef); - return; - } - if (lcl->OperIsScalarLocal()) { // Mark remainder and all fields. @@ -219,9 +211,10 @@ void PromotionLiveness::MarkUseDef(GenTreeLclVarCommon* lcl, BitVec& useSet, Bit } else { - unsigned offs = lcl->GetLclOffs(); - unsigned size = lcl->GetLayout(m_compiler)->GetSize(); - size_t index = Promotion::BinarySearch(reps, offs); + unsigned offs = lcl->GetLclOffs(); + unsigned size = GetSizeOfStructLocal(stmt, lcl); + + size_t index = Promotion::BinarySearch(reps, offs); if ((ssize_t)index < 0) { @@ -264,6 +257,30 @@ void PromotionLiveness::MarkUseDef(GenTreeLclVarCommon* lcl, BitVec& useSet, Bit } } +//------------------------------------------------------------------------ +// GetSizeOfStructLocal: +// Get the size of a struct local (either a TYP_STRUCT typed local, or a +// GT_LCL_ADDR retbuf definition). +// +// Parameters: +// stmt - Statement containing the local +// lcl - The local node +// +unsigned PromotionLiveness::GetSizeOfStructLocal(Statement* stmt, GenTreeLclVarCommon* lcl) +{ + if (lcl->OperIs(GT_LCL_ADDR)) + { + // Retbuf definition. Find the definition size from the + // containing call. + Compiler::FindLinkData data = m_compiler->gtFindLink(stmt, lcl); + assert((data.parent != nullptr) && data.parent->IsCall() && + (m_compiler->gtCallGetDefinedRetBufLclAddr(data.parent->AsCall()) == lcl)); + return m_compiler->typGetObjLayout(data.parent->AsCall()->gtRetClsHnd)->GetSize(); + } + + return lcl->GetLayout(m_compiler)->GetSize(); +} + //------------------------------------------------------------------------ // MarkIndex: // Mark specific bits in use/def bit vectors depending on whether this is a use def. @@ -435,7 +452,7 @@ void PromotionLiveness::FillInLiveness() { for (GenTree* cur = stmt->GetTreeListEnd(); cur != nullptr; cur = cur->gtPrev) { - FillInLiveness(life, volatileVars, cur->AsLclVarCommon()); + FillInLiveness(life, volatileVars, stmt, cur->AsLclVarCommon()); } } else @@ -445,7 +462,7 @@ void PromotionLiveness::FillInLiveness() // Skip liveness updates/marking for defs; they may be conditionally executed. if ((cur->gtFlags & GTF_VAR_DEF) == 0) { - FillInLiveness(life, volatileVars, cur->AsLclVarCommon()); + FillInLiveness(life, volatileVars, stmt, cur->AsLclVarCommon()); } } } @@ -467,9 +484,10 @@ void PromotionLiveness::FillInLiveness() // Parameters: // life - The current life set. Will be read and updated depending on 'lcl'. // volatileVars - Bit vector of variables that are live always. +// stmt - Statement containing the local // lcl - The IR node to process liveness for and to mark with liveness information. // -void PromotionLiveness::FillInLiveness(BitVec& life, BitVec volatileVars, GenTreeLclVarCommon* lcl) +void PromotionLiveness::FillInLiveness(BitVec& life, BitVec volatileVars, Statement* stmt, GenTreeLclVarCommon* lcl) { AggregateInfo* agg = m_aggregates.Lookup(lcl->GetLclNum()); if (agg == nullptr) @@ -483,7 +501,7 @@ void PromotionLiveness::FillInLiveness(BitVec& life, BitVec volatileVars, GenTre unsigned baseIndex = m_structLclToTrackedIndex[lcl->GetLclNum()]; var_types accessType = lcl->TypeGet(); - if (accessType == TYP_STRUCT) + if ((accessType == TYP_STRUCT) || lcl->OperIs(GT_LCL_ADDR)) { // We need an external bit set to represent dying fields/remainder on a struct use. BitVecTraits aggTraits(1 + (unsigned)agg->Replacements.size(), m_compiler); @@ -515,7 +533,7 @@ void PromotionLiveness::FillInLiveness(BitVec& life, BitVec volatileVars, GenTre else { unsigned offs = lcl->GetLclOffs(); - unsigned size = lcl->GetLayout(m_compiler)->GetSize(); + unsigned size = GetSizeOfStructLocal(stmt, lcl); size_t index = Promotion::BinarySearch(agg->Replacements, offs); if ((ssize_t)index < 0) @@ -579,25 +597,6 @@ void PromotionLiveness::FillInLiveness(BitVec& life, BitVec volatileVars, GenTre } else { - if (lcl->OperIs(GT_LCL_ADDR)) - { - // Retbuf -- these are definitions but we do not know of how much. - // We never treat them as killing anything, but we do store liveness information for them. - BitVecTraits aggTraits(1 + (unsigned)agg->Replacements.size(), m_compiler); - BitVec aggDeaths(BitVecOps::MakeEmpty(&aggTraits)); - // Copy preexisting liveness information. - for (size_t i = 0; i <= agg->Replacements.size(); i++) - { - unsigned varIndex = baseIndex + (unsigned)i; - if (!BitVecOps::IsMember(m_bvTraits, life, varIndex)) - { - BitVecOps::AddElemD(&aggTraits, aggDeaths, (unsigned)i); - } - } - m_aggDeaths.Set(lcl, aggDeaths); - return; - } - unsigned offs = lcl->GetLclOffs(); size_t index = Promotion::BinarySearch(agg->Replacements, offs); if ((ssize_t)index < 0) From 802d9036152de26ac070d836e5e5ba3e086dc096 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 8 Jul 2024 15:50:19 +0200 Subject: [PATCH 2/2] Nit --- src/coreclr/jit/promotionliveness.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/promotionliveness.cpp b/src/coreclr/jit/promotionliveness.cpp index b482083fbbe24..8f5acc3ebd35d 100644 --- a/src/coreclr/jit/promotionliveness.cpp +++ b/src/coreclr/jit/promotionliveness.cpp @@ -211,10 +211,9 @@ void PromotionLiveness::MarkUseDef(Statement* stmt, GenTreeLclVarCommon* lcl, Bi } else { - unsigned offs = lcl->GetLclOffs(); - unsigned size = GetSizeOfStructLocal(stmt, lcl); - - size_t index = Promotion::BinarySearch(reps, offs); + unsigned offs = lcl->GetLclOffs(); + unsigned size = GetSizeOfStructLocal(stmt, lcl); + size_t index = Promotion::BinarySearch(reps, offs); if ((ssize_t)index < 0) {