diff --git a/src/coreclr/jit/bitsetasshortlong.h b/src/coreclr/jit/bitsetasshortlong.h index 006f66fc178dcb..b6ebbc70a40ed7 100644 --- a/src/coreclr/jit/bitsetasshortlong.h +++ b/src/coreclr/jit/bitsetasshortlong.h @@ -45,6 +45,7 @@ class BitSetOps +void BitSetOps::FillDLong(Env env, BitSetShortLongRep& bs) +{ + assert(!IsShort(env)); + unsigned len = BitSetTraits::GetArrSize(env); + for (unsigned i = 0; i < len; i++) + { + bs[i] = size_t(-1); + } +} + template BitSetShortLongRep BitSetOps, bool> LocalDefinitionsMap; + + FlowGraphNaturalLoops* m_loops; + // For every loop, we track all definitions exclusive to that loop. + // Definitions in descendant loops are not kept in their ancestor's maps. + LocalDefinitionsMap** m_maps; + // Blocks whose IR we have visited to find local definitions in. + BitVec m_visitedBlocks; + + LocalDefinitionsMap* GetOrCreateMap(FlowGraphNaturalLoop* loop); + + template + bool VisitLoopNestMaps(FlowGraphNaturalLoop* loop, TFunc& func); +public: + LoopDefinitions(FlowGraphNaturalLoops* loops); + + template + void VisitDefinedLocalNums(FlowGraphNaturalLoop* loop, TFunc func); +}; + // The following holds information about instr offsets in terms of generated code. enum class IPmappingDscKind diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 7428137448c4a8..55d133468511a6 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -5205,6 +5205,58 @@ BasicBlockVisit FlowGraphNaturalLoop::VisitRegularExitBlocks(TFunc func) return BasicBlockVisit::Continue; } +//------------------------------------------------------------------------------ +// LoopDefinitions:VisitLoopNestMaps: +// Visit all occurrence maps of the specified loop nest. +// +// Type parameters: +// TFunc - bool(LocalToOccurrenceMap*) functor that returns true to continue +// the visit and false to abort. +// +// Parameters: +// loop - Root loop of the nest. +// func - Functor instance +// +// Returns: +// True if the visit completed; false if "func" returned false for any map. +// +template +bool LoopDefinitions::VisitLoopNestMaps(FlowGraphNaturalLoop* loop, TFunc& func) +{ + for (FlowGraphNaturalLoop* child = loop->GetChild(); child != nullptr; child = child->GetSibling()) + { + if (!VisitLoopNestMaps(child, func)) + { + return false; + } + } + + return func(GetOrCreateMap(loop)); +} + +//------------------------------------------------------------------------------ +// LoopDefinitions:VisitDefinedLocalNums: +// Call a callback for all locals that are defined in the specified loop. +// +// Parameters: +// loop - The loop +// func - The callback +// +template +void LoopDefinitions::VisitDefinedLocalNums(FlowGraphNaturalLoop* loop, TFunc func) +{ + auto visit = [=, &func](LocalDefinitionsMap* map) { + for (unsigned lclNum : LocalDefinitionsMap::KeyIteration(map)) + { + func(lclNum); + } + + return true; + }; + + VisitLoopNestMaps(loop, visit); +} + /*****************************************************************************/ #endif //_COMPILER_HPP_ /*****************************************************************************/ diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 4d1b2697215324..e3cac6ad8bb2fb 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -175,29 +175,6 @@ void Compiler::fgSequenceLocals(Statement* stmt) seq.Sequence(stmt); } -// Data structure that keeps track of local definitions inside loops. -class LoopDefinitions -{ - typedef JitHashTable, bool> LocalDefinitionsMap; - - FlowGraphNaturalLoops* m_loops; - // For every loop, we track all definitions exclusive to that loop. - // Definitions in descendant loops are not kept in their ancestor's maps. - LocalDefinitionsMap** m_maps; - // Blocks whose IR we have visited to find local definitions in. - BitVec m_visitedBlocks; - - LocalDefinitionsMap* GetOrCreateMap(FlowGraphNaturalLoop* loop); - - template - bool VisitLoopNestMaps(FlowGraphNaturalLoop* loop, TFunc& func); -public: - LoopDefinitions(FlowGraphNaturalLoops* loops); - - template - void VisitDefinedLocalNums(FlowGraphNaturalLoop* loop, TFunc func); -}; - LoopDefinitions::LoopDefinitions(FlowGraphNaturalLoops* loops) : m_loops(loops) { @@ -263,7 +240,7 @@ LoopDefinitions::LocalDefinitionsMap* LoopDefinitions::GetOrCreateMap(FlowGraphN fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) { GenTreeLclVarCommon* lcl = (*use)->AsLclVarCommon(); - if (!lcl->OperIsLocalStore()) + if (!lcl->OperIsLocalStore() && !lcl->OperIs(GT_LCL_ADDR)) { return Compiler::WALK_CONTINUE; } @@ -320,58 +297,6 @@ LoopDefinitions::LocalDefinitionsMap* LoopDefinitions::GetOrCreateMap(FlowGraphN return map; } -//------------------------------------------------------------------------------ -// LoopDefinitions:VisitLoopNestMaps: -// Visit all occurrence maps of the specified loop nest. -// -// Type parameters: -// TFunc - bool(LocalToOccurrenceMap*) functor that returns true to continue -// the visit and false to abort. -// -// Parameters: -// loop - Root loop of the nest. -// func - Functor instance -// -// Returns: -// True if the visit completed; false if "func" returned false for any map. -// -template -bool LoopDefinitions::VisitLoopNestMaps(FlowGraphNaturalLoop* loop, TFunc& func) -{ - for (FlowGraphNaturalLoop* child = loop->GetChild(); child != nullptr; child = child->GetSibling()) - { - if (!VisitLoopNestMaps(child, func)) - { - return false; - } - } - - return func(GetOrCreateMap(loop)); -} - -//------------------------------------------------------------------------------ -// LoopDefinitions:VisitDefinedLocalNums: -// Call a callback for all locals that are defined in the specified loop. -// -// Parameters: -// loop - The loop -// func - The callback -// -template -void LoopDefinitions::VisitDefinedLocalNums(FlowGraphNaturalLoop* loop, TFunc func) -{ - auto visit = [=, &func](LocalDefinitionsMap* map) { - for (unsigned lclNum : LocalDefinitionsMap::KeyIteration(map)) - { - func(lclNum); - } - - return true; - }; - - VisitLoopNestMaps(loop, visit); -} - struct LocalEqualsLocalAddrAssertion { // Local num on the LHS diff --git a/src/coreclr/jit/promotion.cpp b/src/coreclr/jit/promotion.cpp index e695a32a0c82b2..bf217a454a1ba5 100644 --- a/src/coreclr/jit/promotion.cpp +++ b/src/coreclr/jit/promotion.cpp @@ -1625,11 +1625,32 @@ GenTree* Promotion::CreateReadBack(Compiler* compiler, unsigned structLclNum, co return store; } +ReplaceVisitor::ReplaceVisitor(Promotion* prom, AggregateInfoMap& aggregates, PromotionLiveness* liveness, LoopDefinitions* loopDefs) + : GenTreeVisitor(prom->m_compiler) + , m_promotion(prom) + , m_aggregates(aggregates) + , m_liveness(liveness) + , m_loopDefs(loopDefs) + , m_replacementTraits(0, prom->m_compiler) +{ + unsigned numReplacements = 0; + for (AggregateInfo* agg : m_aggregates) + { + for (Replacement& rep : agg->Replacements) + { + rep.VarIndex = numReplacements++; + } + } + + m_replacementTraits = BitVecTraits(numReplacements, m_compiler); + m_writeBacksNeededOut = new (m_compiler, CMK_Promotion) BitVec[m_compiler->m_dfsTree->GetPostOrderCount()] {}; +} + //------------------------------------------------------------------------ // StartBlock: // Handle reaching the end of the currently started block by preparing // internal state for upcoming basic blocks, and inserting any necessary -// readbacks. +// readbacks. Must be called in RPO. // // Parameters: // block - The block @@ -1642,20 +1663,59 @@ Statement* ReplaceVisitor::StartBlock(BasicBlock* block) m_currentBlock = block; #ifdef DEBUG - // At the start of every block we expect all replacements to be in their - // local home. + // At the start of every block we expect all replacements to be up to date in their local. for (AggregateInfo* agg : m_aggregates) { for (Replacement& rep : agg->Replacements) { assert(!rep.NeedsReadBack); - assert(rep.NeedsWriteBack); } } assert(m_numPendingReadBacks == 0); #endif + // Find the set of variables that need write-backs. + m_writeBacksNeeded = BitVecOps::MakeEmpty(&m_replacementTraits); + + FlowGraphNaturalLoop* loop = nullptr; + for (FlowEdge* edge = m_compiler->BlockPredsWithEH(block); edge != nullptr; edge = edge->getNextPredEdge()) + { + BasicBlock* pred = edge->getSourceBlock(); + if (pred->bbPostorderNum <= block->bbPostorderNum) + { + loop = m_compiler->m_loops->GetLoopByHeader(block); + + if (loop != nullptr) + { + // Ignore the loop backedge. We will set write backs for + // defined locals after processing other preds. + continue; + } + + // Non natural loop backedge. Go conservative. + BitVecOps::FillD(&m_replacementTraits, m_writeBacksNeeded); + continue; + } + + const BitVec& predBitVec = m_writeBacksNeededOut[pred->bbPostorderNum]; + BitVecOps::UnionD(&m_replacementTraits, m_writeBacksNeeded, predBitVec); + } + + if (loop != nullptr) + { + m_loopDefs->VisitDefinedLocalNums(loop, [=](unsigned lclNum) { + AggregateInfo* agg = m_aggregates.Lookup(lclNum); + if (agg != nullptr) + { + for (Replacement& rep : agg->Replacements) + { + BitVecOps::AddElemD(&m_replacementTraits, m_writeBacksNeeded, rep.VarIndex); + } + } + }); + } + // OSR locals and parameters may need an initial read back, which we mark // when we start the initial BB. if (block != m_compiler->fgFirstBB) @@ -1726,9 +1786,8 @@ Statement* ReplaceVisitor::StartBlock(BasicBlock* block) // Remarks: // We currently expect all fields to be most up-to-date in their field locals // at the beginning of every basic block. That means all replacements should -// have Replacement::NeedsReadBack == false and Replacement::NeedsWriteBack -// == true at the beginning of every block. This function makes it so that is -// the case. +// have Replacement::NeedsReadBack == false at the beginning of every block. +// This function makes it so that is the case. // void ReplaceVisitor::EndBlock() { @@ -1737,7 +1796,7 @@ void ReplaceVisitor::EndBlock() for (size_t i = 0; i < agg->Replacements.size(); i++) { Replacement& rep = agg->Replacements[i]; - assert(!rep.NeedsReadBack || !rep.NeedsWriteBack); + assert(!rep.NeedsReadBack || !NeedsWriteBack(rep)); if (rep.NeedsReadBack) { if (m_liveness->IsReplacementLiveOut(m_currentBlock, agg->LclNum, (unsigned)i)) @@ -1776,12 +1835,12 @@ void ReplaceVisitor::EndBlock() ClearNeedsReadBack(rep); } - - SetNeedsWriteBack(rep); } } assert(m_numPendingReadBacks == 0); + + m_writeBacksNeededOut[m_currentBlock->bbPostorderNum] = m_writeBacksNeeded; } //------------------------------------------------------------------------ @@ -1849,6 +1908,11 @@ Compiler::fgWalkResult ReplaceVisitor::PostOrderVisit(GenTree** use, GenTree* us return fgWalkResult::WALK_CONTINUE; } +bool ReplaceVisitor::NeedsWriteBack(Replacement& rep) +{ + return BitVecOps::IsMember(&m_replacementTraits, m_writeBacksNeeded, rep.VarIndex); +} + //------------------------------------------------------------------------ // SetNeedsWriteBack: // Track that a replacement is more up-to-date in the field local than the @@ -1860,7 +1924,7 @@ Compiler::fgWalkResult ReplaceVisitor::PostOrderVisit(GenTree** use, GenTree* us // void ReplaceVisitor::SetNeedsWriteBack(Replacement& rep) { - rep.NeedsWriteBack = true; + BitVecOps::AddElemD(&m_replacementTraits, m_writeBacksNeeded, rep.VarIndex); assert(!rep.NeedsReadBack); } @@ -1871,7 +1935,7 @@ void ReplaceVisitor::SetNeedsWriteBack(Replacement& rep) // void ReplaceVisitor::ClearNeedsWriteBack(Replacement& rep) { - rep.NeedsWriteBack = false; + BitVecOps::RemoveElemD(&m_replacementTraits, m_writeBacksNeeded, rep.VarIndex); } //------------------------------------------------------------------------ @@ -2716,12 +2780,15 @@ void ReplaceVisitor::ReplaceLocal(GenTree** use, GenTree* user) lcl->gtFlags |= GTF_VAR_DEATH; CheckForwardSubForLastUse(lclNum); - // Relying on the values in the struct local after this struct use - // would effectively introduce another use of the struct, so - // indicate that no replacements are up to date. - for (Replacement& rep : replacements) + // Call arguments that are marked as dying can be mutated due + // to omission of copies for implicit byrefs. We thus cannot + // assume the struct local is up to date. + //if (user->IsCall()) { - SetNeedsWriteBack(rep); + for (Replacement& rep : replacements) + { + SetNeedsWriteBack(rep); + } } } } @@ -2852,7 +2919,7 @@ void ReplaceVisitor::CheckForwardSubForLastUse(unsigned lclNum) void ReplaceVisitor::WriteBackBeforeCurrentStatement(unsigned lcl, unsigned offs, unsigned size) { VisitOverlappingReplacements(lcl, offs, size, [this, lcl](Replacement& rep) { - if (!rep.NeedsWriteBack) + if (!NeedsWriteBack(rep)) { return true; } @@ -2881,7 +2948,7 @@ void ReplaceVisitor::WriteBackBeforeCurrentStatement(unsigned lcl, unsigned offs void ReplaceVisitor::WriteBackBeforeUse(GenTree** use, unsigned lcl, unsigned offs, unsigned size) { VisitOverlappingReplacements(lcl, offs, size, [this, &use, lcl](Replacement& rep) { - if (!rep.NeedsWriteBack) + if (!NeedsWriteBack(rep)) { return true; } @@ -3006,20 +3073,31 @@ PhaseStatus Promotion::Run() return PhaseStatus::MODIFIED_NOTHING; } - // We should have a proper entry BB where we can put IR that will only be - // run once into. This is a precondition of the phase. - assert(m_compiler->fgFirstBB->bbPreds == nullptr); - // Compute liveness for the fields and remainders. PromotionLiveness liveness(m_compiler, aggregates); liveness.Run(); + // Compute DFS, loops and loop definitions to use for flow-sensitive + // knowledge about when the stack home is up to date. + if (m_compiler->m_dfsTree == nullptr) + { + m_compiler->m_dfsTree = m_compiler->fgComputeDfs(); + } + + if (m_compiler->m_loops == nullptr) + { + m_compiler->m_loops = FlowGraphNaturalLoops::Find(m_compiler->m_dfsTree); + } + + LoopDefinitions loopDefs(m_compiler->m_loops); + JITDUMP("Making replacements\n\n"); // Make all replacements we decided on. - ReplaceVisitor replacer(this, aggregates, &liveness); - for (BasicBlock* bb : m_compiler->Blocks()) + ReplaceVisitor replacer(this, aggregates, &liveness, &loopDefs); + for (unsigned i = m_compiler->m_dfsTree->GetPostOrderCount(); i != 0; i--) { + BasicBlock* bb = m_compiler->m_dfsTree->GetPostOrder(i - 1); Statement* firstStmt = replacer.StartBlock(bb); JITDUMP("\nReplacing in "); diff --git a/src/coreclr/jit/promotion.h b/src/coreclr/jit/promotion.h index 4fca7bdf9ca1fa..b233bb17ff4c3e 100644 --- a/src/coreclr/jit/promotion.h +++ b/src/coreclr/jit/promotion.h @@ -20,8 +20,8 @@ struct Replacement unsigned Offset; var_types AccessType; unsigned LclNum = BAD_VAR_NUM; - // Is the replacement local (given by LclNum) fresher than the value in the struct local? - bool NeedsWriteBack = true; + // Index of this replacement into write-back bit vector. + unsigned VarIndex = UINT_MAX; // Is the value in the struct local fresher than the replacement local? // Note that the invariant is that this is always false at the entrance to // a basic block, i.e. all predecessors would have read the replacement @@ -242,11 +242,16 @@ class ReplaceVisitor : public GenTreeVisitor 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; + LoopDefinitions* m_loopDefs; + bool m_madeChanges = false; + unsigned m_numPendingReadBacks = 0; + bool m_mayHaveForwardSub = false; + Statement* m_currentStmt = nullptr; + BasicBlock* m_currentBlock = nullptr; + + BitVecTraits m_replacementTraits; + BitVec* m_writeBacksNeededOut = nullptr; + BitVec m_writeBacksNeeded; public: enum @@ -256,13 +261,7 @@ class ReplaceVisitor : public GenTreeVisitor ComputeStack = true, }; - ReplaceVisitor(Promotion* prom, AggregateInfoMap& aggregates, PromotionLiveness* liveness) - : GenTreeVisitor(prom->m_compiler) - , m_promotion(prom) - , m_aggregates(aggregates) - , m_liveness(liveness) - { - } + ReplaceVisitor(Promotion* prom, AggregateInfoMap& aggregates, PromotionLiveness* liveness, LoopDefinitions* loopDefs); bool MadeChanges() { @@ -281,6 +280,7 @@ class ReplaceVisitor : public GenTreeVisitor fgWalkResult PostOrderVisit(GenTree** use, GenTree* user); private: + bool NeedsWriteBack(Replacement& rep); void SetNeedsWriteBack(Replacement& rep); void ClearNeedsWriteBack(Replacement& rep); void SetNeedsReadBack(Replacement& rep); diff --git a/src/coreclr/jit/promotiondecomposition.cpp b/src/coreclr/jit/promotiondecomposition.cpp index 169de3bcceb970..94f8604e9dba78 100644 --- a/src/coreclr/jit/promotiondecomposition.cpp +++ b/src/coreclr/jit/promotiondecomposition.cpp @@ -496,7 +496,7 @@ class DecompositionPlan if ((entry.FromReplacement != nullptr) && (entry.Type == TYP_REF)) { Replacement* rep = entry.FromReplacement; - if (rep->NeedsWriteBack) + if (m_replacer->NeedsWriteBack(*rep)) { statements->AddStatement( Promotion::CreateWriteBack(m_compiler, m_src->AsLclVarCommon()->GetLclNum(), *rep)); @@ -784,7 +784,7 @@ class DecompositionPlan if (entry.FromReplacement != nullptr) { // Check if the remainder is going to handle it. - if ((remainderStrategy.Type == RemainderStrategy::FullBlock) && !entry.FromReplacement->NeedsWriteBack && + if ((remainderStrategy.Type == RemainderStrategy::FullBlock) && !m_replacer->NeedsWriteBack(*entry.FromReplacement) && (entry.ToReplacement == nullptr)) { #ifdef DEBUG @@ -1320,7 +1320,7 @@ void ReplaceVisitor::HandleStructStore(GenTree** use, GenTree* user) JITDUMP("*** Block operation partially overlaps with start replacement of destination V%02u (%s)\n", dstFirstRep->LclNum, dstFirstRep->Description); - if (dstFirstRep->NeedsWriteBack) + if (NeedsWriteBack(*dstFirstRep)) { // The value of the replacement will be partially assembled from its old value and this struct // operation. @@ -1344,7 +1344,7 @@ void ReplaceVisitor::HandleStructStore(GenTree** use, GenTree* user) JITDUMP("*** Block operation partially overlaps with end replacement of destination V%02u (%s)\n", dstLastRep->LclNum, dstLastRep->Description); - if (dstLastRep->NeedsWriteBack) + if (NeedsWriteBack(*dstLastRep)) { result.AddStatement(Promotion::CreateWriteBack(m_compiler, dstLcl->GetLclNum(), *dstLastRep)); ClearNeedsWriteBack(*dstLastRep); @@ -1368,7 +1368,7 @@ void ReplaceVisitor::HandleStructStore(GenTree** use, GenTree* user) JITDUMP("*** Block operation partially overlaps with start replacement of source V%02u (%s)\n", srcFirstRep->LclNum, srcFirstRep->Description); - if (srcFirstRep->NeedsWriteBack) + if (NeedsWriteBack(*srcFirstRep)) { result.AddStatement(Promotion::CreateWriteBack(m_compiler, srcLcl->GetLclNum(), *srcFirstRep)); ClearNeedsWriteBack(*srcFirstRep); @@ -1385,7 +1385,7 @@ void ReplaceVisitor::HandleStructStore(GenTree** use, GenTree* user) JITDUMP("*** Block operation partially overlaps with end replacement of source V%02u (%s)\n", srcLastRep->LclNum, srcLastRep->Description); - if (srcLastRep->NeedsWriteBack) + if (NeedsWriteBack(*srcLastRep)) { result.AddStatement(Promotion::CreateWriteBack(m_compiler, srcLcl->GetLclNum(), *srcLastRep)); ClearNeedsWriteBack(*srcLastRep); @@ -1627,7 +1627,7 @@ void ReplaceVisitor::CopyBetweenFields(GenTree* store, assert(srcLcl != nullptr); statements->AddStatement(Promotion::CreateReadBack(m_compiler, srcLcl->GetLclNum(), *srcRep)); ClearNeedsReadBack(*srcRep); - assert(!srcRep->NeedsWriteBack); + assert(!NeedsWriteBack(*srcRep)); } if ((dstRep < dstEndRep) && (srcRep < srcEndRep))