From 4c6e8cf16e6f067ae11443803277519c85ad752b Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sat, 24 Jun 2023 15:35:32 +0200 Subject: [PATCH 1/2] JIT: Optimize aggregate info iteration in physical promotion In several places we have to iterate over all promoted aggregates. This was less efficient than it has to be because we stored aggregate information in a flat array indexed by local number, and the vast majority of locals are not physically promoted. This change introduces an AggregateInfoMap that stores just the promoted aggregates, and additionally stores an array mapping locals to the promoted aggregates. This does mean that we need a two level lookup to map a local number back to its aggregate information, but that still seems better for throughput than the previous behavior. --- src/coreclr/jit/promotion.cpp | 177 ++++++++++++--------- src/coreclr/jit/promotion.h | 63 +++++--- src/coreclr/jit/promotiondecomposition.cpp | 48 +++--- src/coreclr/jit/promotionliveness.cpp | 22 +-- 4 files changed, 174 insertions(+), 136 deletions(-) diff --git a/src/coreclr/jit/promotion.cpp b/src/coreclr/jit/promotion.cpp index b2fd63213b4604..3eeed9f8bc66e7 100644 --- a/src/coreclr/jit/promotion.cpp +++ b/src/coreclr/jit/promotion.cpp @@ -211,6 +211,65 @@ bool AggregateInfo::OverlappingReplacements(unsigned offset, return true; } +//------------------------------------------------------------------------ +// AggregateInfoMap::AggregateInfoMap: +// Construct a map that maps locals to AggregateInfo. +// +// Parameters: +// allocator - The allocator +// numLocals - Number of locals to support in the map +// +AggregateInfoMap::AggregateInfoMap(CompAllocator allocator, unsigned numLocals) + : Aggregates(allocator), m_numLocals(numLocals) +{ + m_lclNumToAggregateIndex = new (allocator) unsigned[numLocals]; + for (unsigned i = 0; i < numLocals; i++) + { + m_lclNumToAggregateIndex[i] = UINT_MAX; + } +} + +//------------------------------------------------------------------------ +// AggregateInfoMap::Add: +// Add information about a physically promoted aggregate to the map. +// +// Parameters: +// agg - The entry to add +// +void AggregateInfoMap::Add(AggregateInfo* agg) +{ + assert(agg->LclNum < m_numLocals); + assert(m_lclNumToAggregateIndex[agg->LclNum] == UINT_MAX); + + m_lclNumToAggregateIndex[agg->LclNum] = static_cast(Aggregates.size()); + Aggregates.push_back(agg); +} + +//------------------------------------------------------------------------ +// AggregateInfoMap::Lookup: +// Lookup the promotion information for a local. +// +// Parameters: +// lclNum - The local number +// +// Returns: +// Pointer to the aggregate information, or nullptr if the local is not +// physically promoted. +// +AggregateInfo* AggregateInfoMap::Lookup(unsigned lclNum) +{ + assert(lclNum < m_numLocals); + unsigned index = m_lclNumToAggregateIndex[lclNum]; + + if (index == UINT_MAX) + { + return nullptr; + } + + assert(Aggregates.size() > index); + return Aggregates[index]; +} + struct PrimitiveAccess { unsigned Count = 0; @@ -368,8 +427,8 @@ class LocalUses access = &*m_inducedAccesses.insert(m_inducedAccesses.begin() + index, PrimitiveAccess(offs, accessType)); } + access->Count++; access->CountWtd += weight; - INDEBUG(access->Count++); } //------------------------------------------------------------------------ @@ -380,24 +439,22 @@ class LocalUses // Parameters: // comp - Compiler instance // lclNum - Local num for this struct local - // aggregateInfo - [out] Pointer to aggregate info to create and insert replacements into. + // aggregates - Map to add aggregate information into if promotion was done // // Returns: - // Number of promotions picked. + // Number of promotions picked. If above 0, an entry was added to aggregates. // - int PickPromotions(Compiler* comp, unsigned lclNum, AggregateInfo** aggregateInfo) + int PickPromotions(Compiler* comp, unsigned lclNum, AggregateInfoMap& aggregates) { if (m_accesses.size() <= 0) { return 0; } - AggregateInfo*& agg = *aggregateInfo; - JITDUMP("Picking promotions for V%02u\n", lclNum); - assert(agg == nullptr); - int numReps = 0; + AggregateInfo* agg = nullptr; + int numReps = 0; for (size_t i = 0; i < m_accesses.size(); i++) { const Access& access = m_accesses[i]; @@ -415,6 +472,7 @@ class LocalUses if (agg == nullptr) { agg = new (comp, CMK_Promotion) AggregateInfo(comp->getAllocator(CMK_Promotion), lclNum); + aggregates.Add(agg); } agg->Replacements.push_back(Replacement(access.Offset, access.AccessType)); @@ -440,19 +498,19 @@ class LocalUses // Parameters: // comp - Compiler instance // lclNum - Local num for this struct local - // aggregateInfo - [out] Pointer to aggregate info to create and insert replacements into. + // aggregates - Map for aggregate information // // Returns: // Number of new promotions. // - int PickInducedPromotions(Compiler* comp, unsigned lclNum, AggregateInfo** aggregateInfo) + int PickInducedPromotions(Compiler* comp, unsigned lclNum, AggregateInfoMap& aggregates) { if (m_inducedAccesses.size() <= 0) { return 0; } - AggregateInfo*& agg = *aggregateInfo; + AggregateInfo* agg = aggregates.Lookup(lclNum); if ((agg != nullptr) && (agg->Replacements.size() >= PHYSICAL_PROMOTION_MAX_PROMOTIONS_PER_STRUCT)) { @@ -511,6 +569,7 @@ class LocalUses if (agg == nullptr) { agg = new (comp, CMK_Promotion) AggregateInfo(comp->getAllocator(CMK_Promotion), lclNum); + aggregates.Add(agg); } size_t insertionIndex; @@ -958,15 +1017,14 @@ class LocalsUseVisitor : public GenTreeVisitor // struct with promotions. // // Parameters: - // aggregates - Appropriately sized vector to create aggregate information in. + // aggregates - Map for aggregates // // Returns: // True if any struct was physically promoted with at least one replacement; // otherwise false. // - bool PickPromotions(jitstd::vector& aggregates) + bool PickPromotions(AggregateInfoMap& aggregates) { - unsigned numLocals = (unsigned)aggregates.size(); JITDUMP("Picking promotions\n"); int totalNumPromotions = 0; @@ -985,7 +1043,7 @@ class LocalsUseVisitor : public GenTreeVisitor // fine to avoid the pathological cases. const int maxTotalNumPromotions = JitConfig.JitMaxLocalsToTrack(); - for (unsigned lclNum = 0; lclNum < numLocals; lclNum++) + for (unsigned lclNum = 0; lclNum < m_compiler->lvaCount; lclNum++) { LocalUses* uses = m_uses[lclNum]; if (uses == nullptr) @@ -1000,7 +1058,7 @@ class LocalsUseVisitor : public GenTreeVisitor } #endif - totalNumPromotions += uses->PickPromotions(m_compiler, lclNum, &aggregates[lclNum]); + totalNumPromotions += uses->PickPromotions(m_compiler, lclNum, aggregates); if (totalNumPromotions >= maxTotalNumPromotions) { @@ -1061,7 +1119,7 @@ class LocalsUseVisitor : public GenTreeVisitor } bool again = false; - for (unsigned lclNum = 0; lclNum < numLocals; lclNum++) + for (unsigned lclNum = 0; lclNum < m_compiler->lvaCount; lclNum++) { LocalUses* uses = m_uses[lclNum]; if (uses == nullptr) @@ -1075,7 +1133,7 @@ class LocalsUseVisitor : public GenTreeVisitor } #endif - int numInducedProms = uses->PickInducedPromotions(m_compiler, lclNum, &aggregates[lclNum]); + int numInducedProms = uses->PickInducedPromotions(m_compiler, lclNum, aggregates); again |= numInducedProms > 0; totalNumPromotions += numInducedProms; @@ -1093,7 +1151,7 @@ class LocalsUseVisitor : public GenTreeVisitor break; } - for (unsigned lclNum = 0; lclNum < numLocals; lclNum++) + for (unsigned lclNum = 0; lclNum < m_compiler->lvaCount; lclNum++) { if (m_uses[lclNum] != nullptr) { @@ -1110,11 +1168,6 @@ class LocalsUseVisitor : public GenTreeVisitor for (AggregateInfo* agg : aggregates) { - if (agg == nullptr) - { - continue; - } - jitstd::vector& reps = agg->Replacements; assert(reps.size() > 0); @@ -1207,10 +1260,10 @@ class LocalsUseVisitor : public GenTreeVisitor // may induce new LCL_FLD nodes in the candidate. // block - The block that the assignment appears in. // - void InduceAccessesFromRegularlyPromotedStruct(jitstd::vector& aggregates, - GenTreeLclVarCommon* candidateLcl, - GenTreeLclVarCommon* regPromLcl, - BasicBlock* block) + void InduceAccessesFromRegularlyPromotedStruct(AggregateInfoMap& aggregates, + GenTreeLclVarCommon* candidateLcl, + GenTreeLclVarCommon* regPromLcl, + BasicBlock* block) { unsigned regPromOffs = regPromLcl->GetLclOffs(); unsigned candidateOffs = candidateLcl->GetLclOffs(); @@ -1242,16 +1295,16 @@ class LocalsUseVisitor : public GenTreeVisitor // inducer - The local node that may induce new LCL_FLD nodes in the candidate. // block - The block that the assignment appears in. // - void InduceAccessesInCandidate(jitstd::vector& aggregates, - GenTreeLclVarCommon* candidate, - GenTreeLclVarCommon* inducer, - BasicBlock* block) + void InduceAccessesInCandidate(AggregateInfoMap& aggregates, + GenTreeLclVarCommon* candidate, + GenTreeLclVarCommon* inducer, + BasicBlock* block) { unsigned candOffs = candidate->GetLclOffs(); unsigned inducerOffs = inducer->GetLclOffs(); unsigned size = candidate->GetLayout(m_compiler)->GetSize(); - AggregateInfo* inducerAgg = aggregates[inducer->GetLclNum()]; + AggregateInfo* inducerAgg = aggregates.Lookup(inducer->GetLclNum()); if (inducerAgg != nullptr) { Replacement* firstRep; @@ -1283,10 +1336,9 @@ class LocalsUseVisitor : public GenTreeVisitor // type - Type of the induced access. // block - The block with the induced access. // - void InduceAccess( - jitstd::vector& aggregates, unsigned lclNum, unsigned offset, var_types type, BasicBlock* block) + void InduceAccess(AggregateInfoMap& aggregates, unsigned lclNum, unsigned offset, var_types type, BasicBlock* block) { - AggregateInfo* agg = aggregates[lclNum]; + AggregateInfo* agg = aggregates.Lookup(lclNum); if (agg != nullptr) { Replacement* overlapRep; @@ -1808,11 +1860,6 @@ void ReplaceVisitor::StartBlock(BasicBlock* block) // local home. for (AggregateInfo* agg : m_aggregates) { - if (agg == nullptr) - { - continue; - } - for (Replacement& rep : agg->Replacements) { assert(!rep.NeedsReadBack); @@ -1832,11 +1879,6 @@ void ReplaceVisitor::StartBlock(BasicBlock* block) for (AggregateInfo* agg : m_aggregates) { - if (agg == nullptr) - { - continue; - } - LclVarDsc* dsc = m_compiler->lvaGetDesc(agg->LclNum); if (!dsc->lvIsParam && !dsc->lvIsOSRLocal) { @@ -1880,11 +1922,6 @@ void ReplaceVisitor::EndBlock() { for (AggregateInfo* agg : m_aggregates) { - if (agg == nullptr) - { - continue; - } - for (size_t i = 0; i < agg->Replacements.size(); i++) { Replacement& rep = agg->Replacements[i]; @@ -2090,7 +2127,7 @@ void ReplaceVisitor::InsertPreStatementReadBacks() continue; } - AggregateInfo* agg = m_aggregates[lcl->GetLclNum()]; + AggregateInfo* agg = m_aggregates.Lookup(lcl->GetLclNum()); if (agg == nullptr) { continue; @@ -2131,12 +2168,13 @@ void ReplaceVisitor::InsertPreStatementReadBacks() template void ReplaceVisitor::VisitOverlappingReplacements(unsigned lcl, unsigned offs, unsigned size, Func func) { - if (m_aggregates[lcl] == nullptr) + AggregateInfo* agg = m_aggregates.Lookup(lcl); + if (agg == nullptr) { return; } - jitstd::vector& replacements = m_aggregates[lcl]->Replacements; + jitstd::vector& replacements = agg->Replacements; size_t index = Promotion::BinarySearch(replacements, offs); if ((ssize_t)index < 0) @@ -2262,11 +2300,6 @@ GenTree** ReplaceVisitor::InsertMidTreeReadBacks(GenTree** use) for (AggregateInfo* agg : m_aggregates) { - if (agg == nullptr) - { - continue; - } - for (Replacement& rep : agg->Replacements) { if (!rep.NeedsReadBack) @@ -2341,7 +2374,9 @@ bool ReplaceVisitor::IsPromotedStructLocalDying(GenTreeLclVarCommon* lcl) return false; } - AggregateInfo* agg = m_aggregates[lcl->GetLclNum()]; + AggregateInfo* agg = m_aggregates.Lookup(lcl->GetLclNum()); + assert(agg != nullptr); + for (Replacement& rep : agg->Replacements) { if (rep.NeedsReadBack) @@ -2376,12 +2411,13 @@ void ReplaceVisitor::ReplaceLocal(GenTree** use, GenTree* user) { GenTreeLclVarCommon* lcl = (*use)->AsLclVarCommon(); unsigned lclNum = lcl->GetLclNum(); - if (m_aggregates[lclNum] == nullptr) + AggregateInfo* agg = m_aggregates.Lookup(lclNum); + if (agg == nullptr) { return; } - jitstd::vector& replacements = m_aggregates[lclNum]->Replacements; + jitstd::vector& replacements = agg->Replacements; unsigned offs = lcl->GetLclOffs(); var_types accessType = lcl->TypeGet(); @@ -2609,13 +2645,14 @@ void ReplaceVisitor::MarkForReadBack(GenTreeLclVarCommon* lcl, unsigned size DEB // in the (relative) near future. assert(m_compiler->fgGetTopLevelQmark(m_currentStmt->GetRootNode()) == nullptr); - if (m_aggregates[lcl->GetLclNum()] == nullptr) + AggregateInfo* agg = m_aggregates.Lookup(lcl->GetLclNum()); + if (agg == nullptr) { return; } unsigned offs = lcl->GetLclOffs(); - jitstd::vector& replacements = m_aggregates[lcl->GetLclNum()]->Replacements; + jitstd::vector& replacements = agg->Replacements; size_t index = Promotion::BinarySearch(replacements, offs); if ((ssize_t)index < 0) @@ -2693,7 +2730,7 @@ PhaseStatus Promotion::Run() } // Pick promotions based on the use information we just collected. - jitstd::vector aggregates(m_compiler->lvaCount, nullptr, m_compiler->getAllocator(CMK_Promotion)); + AggregateInfoMap aggregates(m_compiler->getAllocator(CMK_Promotion), m_compiler->lvaCount); if (!localsUse.PickPromotions(aggregates)) { // No promotions picked. @@ -2704,11 +2741,6 @@ PhaseStatus Promotion::Run() // to the function. for (AggregateInfo* agg : aggregates) { - if (agg == nullptr) - { - continue; - } - LclVarDsc* dsc = m_compiler->lvaGetDesc(agg->LclNum); if (dsc->lvIsParam || dsc->lvIsOSRLocal) { @@ -2770,11 +2802,6 @@ PhaseStatus Promotion::Run() Statement* prevStmt = nullptr; for (AggregateInfo* agg : aggregates) { - if (agg == nullptr) - { - continue; - } - LclVarDsc* dsc = m_compiler->lvaGetDesc(agg->LclNum); if (dsc->lvSuppressedZeroInit) { diff --git a/src/coreclr/jit/promotion.h b/src/coreclr/jit/promotion.h index 9faa515ec67bf2..acad81872f2cdf 100644 --- a/src/coreclr/jit/promotion.h +++ b/src/coreclr/jit/promotion.h @@ -103,6 +103,29 @@ struct AggregateInfo Replacement** endReplacement); }; +// Map that stores information about promotions made for each local. +class AggregateInfoMap +{ + jitstd::vector Aggregates; + unsigned m_numLocals; + unsigned* m_lclNumToAggregateIndex; + +public: + AggregateInfoMap(CompAllocator allocator, unsigned numLocals); + void Add(AggregateInfo* agg); + AggregateInfo* Lookup(unsigned lclNum); + + jitstd::vector::iterator begin() + { + return Aggregates.begin(); + } + + jitstd::vector::iterator end() + { + return Aggregates.end(); + } +}; + typedef JitHashTable, class StructSegments*> ClassLayoutStructSegmentsMap; class Promotion @@ -150,7 +173,7 @@ class Promotion size_t mid = min + (max - min) / 2; if (vec[mid].*field == offset) { - while (mid > 0 && vec[mid - 1].*field == offset) + while ((mid > 0) && (vec[mid - 1].*field == offset)) { mid--; } @@ -210,19 +233,19 @@ struct BasicBlockLiveness; // Class to compute and track liveness information pertaining promoted structs. class PromotionLiveness { - Compiler* m_compiler; - jitstd::vector& m_aggregates; - BitVecTraits* m_bvTraits = nullptr; - unsigned* m_structLclToTrackedIndex = nullptr; - unsigned m_numVars = 0; - BasicBlockLiveness* m_bbInfo = nullptr; - bool m_hasPossibleBackEdge = false; - BitVec m_liveIn; - BitVec m_ehLiveVars; + Compiler* m_compiler; + AggregateInfoMap& m_aggregates; + BitVecTraits* m_bvTraits = nullptr; + unsigned* m_structLclToTrackedIndex = nullptr; + unsigned m_numVars = 0; + BasicBlockLiveness* m_bbInfo = nullptr; + bool m_hasPossibleBackEdge = false; + BitVec m_liveIn; + BitVec m_ehLiveVars; JitHashTable, BitVec> m_aggDeaths; public: - PromotionLiveness(Compiler* compiler, jitstd::vector& aggregates) + PromotionLiveness(Compiler* compiler, AggregateInfoMap& aggregates) : m_compiler(compiler), m_aggregates(aggregates), m_aggDeaths(compiler->getAllocator(CMK_Promotion)) { } @@ -253,14 +276,14 @@ class ReplaceVisitor : public GenTreeVisitor { friend class DecompositionPlan; - Promotion* m_promotion; - jitstd::vector& m_aggregates; - PromotionLiveness* m_liveness; - bool m_madeChanges = false; - unsigned m_numPendingReadBacks = 0; - bool m_mayHaveForwardSub = false; - Statement* m_currentStmt = nullptr; - BasicBlock* m_currentBlock = nullptr; + Promotion* m_promotion; + AggregateInfoMap& m_aggregates; + PromotionLiveness* m_liveness; + bool m_madeChanges = false; + unsigned m_numPendingReadBacks = 0; + bool m_mayHaveForwardSub = false; + Statement* m_currentStmt = nullptr; + BasicBlock* m_currentBlock = nullptr; public: enum @@ -270,7 +293,7 @@ class ReplaceVisitor : public GenTreeVisitor ComputeStack = true, }; - ReplaceVisitor(Promotion* prom, jitstd::vector& aggregates, PromotionLiveness* liveness) + ReplaceVisitor(Promotion* prom, AggregateInfoMap& aggregates, PromotionLiveness* liveness) : GenTreeVisitor(prom->m_compiler), m_promotion(prom), m_aggregates(aggregates), m_liveness(liveness) { } diff --git a/src/coreclr/jit/promotiondecomposition.cpp b/src/coreclr/jit/promotiondecomposition.cpp index 55029720375f78..f90f0849484cc0 100644 --- a/src/coreclr/jit/promotiondecomposition.cpp +++ b/src/coreclr/jit/promotiondecomposition.cpp @@ -49,27 +49,27 @@ class DecompositionPlan var_types Type; }; - Promotion* m_promotion; - Compiler* m_compiler; - ReplaceVisitor* m_replacer; - jitstd::vector& m_aggregates; - PromotionLiveness* m_liveness; - GenTree* m_store; - GenTree* m_src; - bool m_dstInvolvesReplacements; - bool m_srcInvolvesReplacements; - ArrayStack m_entries; - bool m_hasNonRemainderUseOfStructLocal = false; + Promotion* m_promotion; + Compiler* m_compiler; + ReplaceVisitor* m_replacer; + AggregateInfoMap& m_aggregates; + PromotionLiveness* m_liveness; + GenTree* m_store; + GenTree* m_src; + bool m_dstInvolvesReplacements; + bool m_srcInvolvesReplacements; + ArrayStack m_entries; + bool m_hasNonRemainderUseOfStructLocal = false; public: - DecompositionPlan(Promotion* prom, - ReplaceVisitor* replacer, - jitstd::vector& aggregates, - PromotionLiveness* liveness, - GenTree* store, - GenTree* src, - bool dstInvolvesReplacements, - bool srcInvolvesReplacements) + DecompositionPlan(Promotion* prom, + ReplaceVisitor* replacer, + AggregateInfoMap& aggregates, + PromotionLiveness* liveness, + GenTree* store, + GenTree* src, + bool dstInvolvesReplacements, + bool srcInvolvesReplacements) : m_promotion(prom) , m_compiler(prom->m_compiler) , m_replacer(replacer) @@ -408,7 +408,7 @@ class DecompositionPlan uint8_t initPattern = GetInitPattern(); StructDeaths deaths = m_liveness->GetDeathsForStructLocal(m_store->AsLclVarCommon()); - AggregateInfo* agg = m_aggregates[m_store->AsLclVarCommon()->GetLclNum()]; + AggregateInfo* agg = m_aggregates.Lookup(m_store->AsLclVarCommon()->GetLclNum()); assert((agg != nullptr) && (agg->Replacements.size() > 0)); Replacement* firstRep = agg->Replacements.data(); @@ -706,7 +706,7 @@ class DecompositionPlan if (entry.FromReplacement != nullptr) { - AggregateInfo* srcAgg = m_aggregates[m_src->AsLclVarCommon()->GetLclNum()]; + AggregateInfo* srcAgg = m_aggregates.Lookup(m_src->AsLclVarCommon()->GetLclNum()); Replacement* firstRep = srcAgg->Replacements.data(); assert((entry.FromReplacement >= firstRep) && (entry.FromReplacement < (firstRep + srcAgg->Replacements.size()))); @@ -866,7 +866,7 @@ class DecompositionPlan // Check if this entry is dying anyway. assert(m_dstInvolvesReplacements); - AggregateInfo* agg = m_aggregates[m_store->AsLclVarCommon()->GetLclNum()]; + AggregateInfo* agg = m_aggregates.Lookup(m_store->AsLclVarCommon()->GetLclNum()); assert((agg != nullptr) && (agg->Replacements.size() > 0)); Replacement* firstRep = agg->Replacements.data(); assert((entry.ToReplacement >= firstRep) && (entry.ToReplacement < (firstRep + agg->Replacements.size()))); @@ -1213,7 +1213,7 @@ bool ReplaceVisitor::OverlappingReplacements(GenTreeLclVarCommon* lcl, Replacement** firstReplacement, Replacement** endReplacement) { - AggregateInfo* agg = m_aggregates[lcl->GetLclNum()]; + AggregateInfo* agg = m_aggregates.Lookup(lcl->GetLclNum()); if (agg == nullptr) { return false; @@ -1335,7 +1335,7 @@ void ReplaceVisitor::InitFields(GenTreeLclVarCommon* dstStore, const char* ReplaceVisitor::LastUseString(GenTreeLclVarCommon* lcl, Replacement* rep) { StructDeaths deaths = m_liveness->GetDeathsForStructLocal(lcl); - AggregateInfo* agg = m_aggregates[lcl->GetLclNum()]; + AggregateInfo* agg = m_aggregates.Lookup(lcl->GetLclNum()); assert(agg != nullptr); Replacement* firstRep = agg->Replacements.data(); assert((rep >= firstRep) && (rep < firstRep + agg->Replacements.size())); diff --git a/src/coreclr/jit/promotionliveness.cpp b/src/coreclr/jit/promotionliveness.cpp index 43b37226144893..daddd1dba2873c 100644 --- a/src/coreclr/jit/promotionliveness.cpp +++ b/src/coreclr/jit/promotionliveness.cpp @@ -72,18 +72,11 @@ struct BasicBlockLiveness // void PromotionLiveness::Run() { - m_structLclToTrackedIndex = new (m_compiler, CMK_Promotion) unsigned[m_aggregates.size()]{}; + m_structLclToTrackedIndex = new (m_compiler, CMK_Promotion) unsigned[m_compiler->lvaCount]{}; unsigned trackedIndex = 0; for (AggregateInfo* agg : m_aggregates) { - if (agg == nullptr) - { - continue; - } - m_structLclToTrackedIndex[agg->LclNum] = trackedIndex; - // TODO: We need a scalability limit on these, we cannot always track - // the remainder and all fields. // Remainder. trackedIndex++; // Fields. @@ -192,7 +185,7 @@ void PromotionLiveness::ComputeUseDefSets() // void PromotionLiveness::MarkUseDef(GenTreeLclVarCommon* lcl, BitVec& useSet, BitVec& defSet) { - AggregateInfo* agg = m_aggregates[lcl->GetLclNum()]; + AggregateInfo* agg = m_aggregates.Lookup(lcl->GetLclNum()); if (agg == nullptr) { return; @@ -575,7 +568,7 @@ void PromotionLiveness::FillInLiveness() // void PromotionLiveness::FillInLiveness(BitVec& life, BitVec volatileVars, GenTreeLclVarCommon* lcl) { - AggregateInfo* agg = m_aggregates[lcl->GetLclNum()]; + AggregateInfo* agg = m_aggregates.Lookup(lcl->GetLclNum()); if (agg == nullptr) { return; @@ -808,13 +801,13 @@ bool PromotionLiveness::IsReplacementLiveOut(BasicBlock* bb, unsigned structLcl, StructDeaths PromotionLiveness::GetDeathsForStructLocal(GenTreeLclVarCommon* lcl) { assert((lcl->TypeIs(TYP_STRUCT) || (lcl->OperIs(GT_LCL_ADDR) && ((lcl->gtFlags & GTF_VAR_DEF) != 0))) && - (m_aggregates[lcl->GetLclNum()] != nullptr)); + (m_aggregates.Lookup(lcl->GetLclNum()) != nullptr)); BitVec aggDeaths; bool found = m_aggDeaths.Lookup(lcl, &aggDeaths); assert(found); unsigned lclNum = lcl->GetLclNum(); - AggregateInfo* aggInfo = m_aggregates[lclNum]; + AggregateInfo* aggInfo = m_aggregates.Lookup(lclNum); return StructDeaths(aggDeaths, (unsigned)aggInfo->Replacements.size()); } @@ -861,11 +854,6 @@ void PromotionLiveness::DumpVarSet(BitVec set, BitVec allVars) const char* sep = ""; for (AggregateInfo* agg : m_aggregates) { - if (agg == nullptr) - { - continue; - } - for (size_t j = 0; j <= agg->Replacements.size(); j++) { unsigned index = (unsigned)(m_structLclToTrackedIndex[agg->LclNum] + j); From 31bdf828f6a91f63d916ca1b3bdae3f5c59ddf3e Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Sat, 24 Jun 2023 15:40:22 +0200 Subject: [PATCH 2/2] Nit --- src/coreclr/jit/promotion.cpp | 10 +++++----- src/coreclr/jit/promotion.h | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/promotion.cpp b/src/coreclr/jit/promotion.cpp index 3eeed9f8bc66e7..daf3f21f13bccf 100644 --- a/src/coreclr/jit/promotion.cpp +++ b/src/coreclr/jit/promotion.cpp @@ -220,7 +220,7 @@ bool AggregateInfo::OverlappingReplacements(unsigned offset, // numLocals - Number of locals to support in the map // AggregateInfoMap::AggregateInfoMap(CompAllocator allocator, unsigned numLocals) - : Aggregates(allocator), m_numLocals(numLocals) + : m_aggregates(allocator), m_numLocals(numLocals) { m_lclNumToAggregateIndex = new (allocator) unsigned[numLocals]; for (unsigned i = 0; i < numLocals; i++) @@ -241,8 +241,8 @@ void AggregateInfoMap::Add(AggregateInfo* agg) assert(agg->LclNum < m_numLocals); assert(m_lclNumToAggregateIndex[agg->LclNum] == UINT_MAX); - m_lclNumToAggregateIndex[agg->LclNum] = static_cast(Aggregates.size()); - Aggregates.push_back(agg); + m_lclNumToAggregateIndex[agg->LclNum] = static_cast(m_aggregates.size()); + m_aggregates.push_back(agg); } //------------------------------------------------------------------------ @@ -266,8 +266,8 @@ AggregateInfo* AggregateInfoMap::Lookup(unsigned lclNum) return nullptr; } - assert(Aggregates.size() > index); - return Aggregates[index]; + assert(m_aggregates.size() > index); + return m_aggregates[index]; } struct PrimitiveAccess diff --git a/src/coreclr/jit/promotion.h b/src/coreclr/jit/promotion.h index acad81872f2cdf..42526af3d3adde 100644 --- a/src/coreclr/jit/promotion.h +++ b/src/coreclr/jit/promotion.h @@ -106,7 +106,7 @@ struct AggregateInfo // Map that stores information about promotions made for each local. class AggregateInfoMap { - jitstd::vector Aggregates; + jitstd::vector m_aggregates; unsigned m_numLocals; unsigned* m_lclNumToAggregateIndex; @@ -117,12 +117,12 @@ class AggregateInfoMap jitstd::vector::iterator begin() { - return Aggregates.begin(); + return m_aggregates.begin(); } jitstd::vector::iterator end() { - return Aggregates.end(); + return m_aggregates.end(); } };