diff --git a/src/coreclr/jit/promotion.cpp b/src/coreclr/jit/promotion.cpp index b42a136b907356..fa912106f57e5f 100644 --- a/src/coreclr/jit/promotion.cpp +++ b/src/coreclr/jit/promotion.cpp @@ -778,16 +778,7 @@ class LocalUses else if (lcl->lvIsParam) { // For parameters, the backend may be able to map it directly from a register. - // Small fields can pack many values into each parameter register, so eagerly - // extracting rarely used fields can add substantial work and register pressure. - // Wider fields naturally limit the number of extractions per register. - // Restrict the credit for small fields with few accesses to target these cases. - const weight_t MIN_RELATIVE_ACCESS_WEIGHT = 0.10; - bool allowBitwiseExtraction = - !varTypeIsSmall(access.AccessType) || - (access.CountWtd + inducedCountWtd) >= MIN_RELATIVE_ACCESS_WEIGHT * comp->fgFirstBB->getBBWeight(comp); - if (Promotion::MapsToParameterRegister(comp, lclNum, access.Offset, access.AccessType, - allowBitwiseExtraction)) + if (Promotion::MapsToParameterRegister(comp, lclNum, access.Offset, access.AccessType)) { // No promotion will result in a store to stack in the prolog. costWithout += COST_STRUCT_ACCESS_CYCLES * comp->fgFirstBB->getBBWeight(comp); @@ -1672,11 +1663,261 @@ GenTree* Promotion::CreateReadBack(Compiler* compiler, unsigned structLclNum, co return store; } +//------------------------------------------------------------------------ +// ReplaceVisitor: +// Initialize the replacement visitor. +// +// Parameters: +// prom - Promotion phase. +// aggregates - Promoted aggregates. +// liveness - Liveness of the replacements. +// dfsTree - Traversal order, including exceptional flow. +// +ReplaceVisitor::ReplaceVisitor(Promotion* prom, + AggregateInfoMap& aggregates, + PromotionLiveness* liveness, + FlowGraphDfsTree* dfsTree) + : GenTreeVisitor(prom->m_compiler) + , m_promotion(prom) + , m_aggregates(aggregates) + , m_liveness(liveness) + , m_dfsTree(dfsTree) + , m_postOrderTraits(dfsTree->PostOrderTraits()) +{ +} + +//------------------------------------------------------------------------ +// PrepareReadBacks: +// Initialize field tracking, compute loop/EH materialization boundaries, +// and plan common readbacks before replacing uses. +// +void ReplaceVisitor::PrepareReadBacks() +{ + FlowGraphDfsTree* dfsTree = m_dfsTree; + unsigned index = 0; + bool hasPlannedReadBackCandidates = false; + for (AggregateInfo* agg : m_aggregates) + { + LclVarDsc* dsc = m_compiler->lvaGetDesc(agg->LclNum); + for (unsigned i = 0; i < agg->Replacements.size(); i++) + { + agg->Replacements[i].ReadBackIndex = index++; + if (!hasPlannedReadBackCandidates && (dsc->lvIsParam || dsc->lvIsOSRLocal) && + m_liveness->IsReplacementLiveIn(m_compiler->fgFirstBB, agg->LclNum, i)) + { + hasPlannedReadBackCandidates = true; + } + } + } + + m_readBackTraits = new (m_compiler, CMK_Promotion) BitVecTraits(index, m_compiler); + m_pendingReadBacks = new (m_compiler, CMK_Promotion) BitVec[dfsTree->GetPostOrderCount()]{}; + m_currentStructFields = new (m_compiler, CMK_Promotion) BitVec[dfsTree->GetPostOrderCount()]{}; + m_processedBlocks = BitVecOps::MakeEmpty(&m_postOrderTraits); + m_requiresAlreadyReadBackOnEntry = BitVecOps::MakeEmpty(&m_postOrderTraits); + m_requiresReadBackOnExit = BitVecOps::MakeEmpty(&m_postOrderTraits); + + for (unsigned i = 0; i < dfsTree->GetPostOrderCount(); i++) + { + BasicBlock* block = dfsTree->GetPostOrder(i); + if (m_compiler->bbIsHandlerBeg(block)) + { + BitVecOps::AddElemD(&m_postOrderTraits, m_requiresAlreadyReadBackOnEntry, block->bbPostorderNum); + } + block->VisitRegularSuccs(m_compiler, [&](BasicBlock* succ) { + if (succ->bbPostorderNum >= block->bbPostorderNum) + { + // This includes irreducible backedge targets: their incoming state + // must be settled before visiting predecessors later in RPO. + BitVecOps::AddElemD(&m_postOrderTraits, m_requiresAlreadyReadBackOnEntry, succ->bbPostorderNum); + } + return BasicBlockVisit::Continue; + }); + } + + // The CFG does not change during replacement. Share boundary decisions + // between the planner and the actual materialization. + for (unsigned i = 0; i < dfsTree->GetPostOrderCount(); i++) + { + if (MustMaterializeReadBacks(dfsTree->GetPostOrder(i))) + { + BitVecOps::AddElemD(&m_postOrderTraits, m_requiresReadBackOnExit, i); + } + } + + if (hasPlannedReadBackCandidates) + { + PlanReadBacks(); + } +} + +//------------------------------------------------------------------------ +// PlanReadBacks: +// Estimate lazy readbacks from liveness use/def sets and choose shared +// materialization points for fields requiring mixed-join reconciliation. +// +// Remarks: +// This is a profitability model, not a correctness analysis. Uses are assumed +// to materialize pending fields, and definitions end the incoming value. +// Replacement retains its exact state transitions and inserts any readbacks +// still required, including after partial writes and at EH boundaries. +// +void ReplaceVisitor::PlanReadBacks() +{ + BasicBlock* entry = m_compiler->fgFirstBB; + FlowGraphDfsTree* placementDfs = m_dfsTree; + FlowGraphDominatorTree* domTree = m_compiler->m_domTree; + BitVec pendingOut = BitVecOps::MakeEmpty(&m_postOrderTraits); + BitVec sites = BitVecOps::MakeEmpty(&m_postOrderTraits); + + for (AggregateInfo* agg : m_aggregates) + { + LclVarDsc* dsc = m_compiler->lvaGetDesc(agg->LclNum); + if (!dsc->lvIsParam && !dsc->lvIsOSRLocal) + { + continue; + } + + for (unsigned i = 0; i < agg->Replacements.size(); i++) + { + Replacement& rep = agg->Replacements[i]; + if (!m_liveness->IsReplacementLiveIn(entry, agg->LclNum, i)) + { + continue; + } + + BitVecOps::ClearD(&m_postOrderTraits, pendingOut); + BitVecOps::ClearD(&m_postOrderTraits, sites); + bool hasReconciliation = false; + for (unsigned j = m_dfsTree->GetPostOrderCount(); j > 0; j--) + { + BasicBlock* block = m_dfsTree->GetPostOrder(j - 1); + if (!m_liveness->IsReplacementLiveIn(block, agg->LclNum, i)) + { + continue; + } + + bool pending = block == entry; + if ((block != entry) && + !BitVecOps::IsMember(&m_postOrderTraits, m_requiresAlreadyReadBackOnEntry, block->bbPostorderNum)) + { + bool anyPending = false; + bool allPending = true; + for (FlowEdge* edge : block->PredEdges()) + { + BasicBlock* pred = edge->getSourceBlock(); + if (!m_dfsTree->Contains(pred)) + { + continue; + } + bool predPending = BitVecOps::IsMember(&m_postOrderTraits, pendingOut, pred->bbPostorderNum); + anyPending |= predPending; + allPending &= predPending; + } + pending = anyPending && allPending; + if (anyPending && !allPending) + { + hasReconciliation = true; + for (FlowEdge* edge : block->PredEdges()) + { + BasicBlock* pred = edge->getSourceBlock(); + if (m_dfsTree->Contains(pred) && + BitVecOps::IsMember(&m_postOrderTraits, pendingOut, pred->bbPostorderNum)) + { + BitVecOps::AddElemD(&m_postOrderTraits, sites, pred->bbPostorderNum); + BitVecOps::RemoveElemD(&m_postOrderTraits, pendingOut, pred->bbPostorderNum); + } + } + } + } + if (pending && m_liveness->IsReplacementUsed(block, agg->LclNum, i)) + { + BitVecOps::AddElemD(&m_postOrderTraits, sites, block->bbPostorderNum); + pending = false; + } + if (pending && m_liveness->IsReplacementDefined(block, agg->LclNum, i)) + { + pending = false; + } + if (pending && m_liveness->IsReplacementLiveOut(block, agg->LclNum, i)) + { + if (BitVecOps::IsMember(&m_postOrderTraits, m_requiresReadBackOnExit, block->bbPostorderNum)) + { + BitVecOps::AddElemD(&m_postOrderTraits, sites, block->bbPostorderNum); + } + else + { + BitVecOps::AddElemD(&m_postOrderTraits, pendingOut, block->bbPostorderNum); + } + } + } + + if (!hasReconciliation || (BitVecOps::Count(&m_postOrderTraits, sites) < 2)) + { + continue; + } + + if (domTree == nullptr) + { + if (entry->bbPostorderNum + 1 != m_dfsTree->GetPostOrderCount()) + { + // Before global morph the DFS may also retain the original + // OSR entry and a disconnected merged return. Dominators + // require a single root; its subtree is a postorder prefix. + placementDfs = new (m_compiler, CMK_Promotion) + FlowGraphDfsTree(m_compiler, m_dfsTree->GetPostOrder(), entry->bbPostorderNum + 1, + m_dfsTree->HasCycle(), m_dfsTree->IsProfileAware()); + } + domTree = FlowGraphDominatorTree::Build(placementDfs); + if (placementDfs == m_dfsTree) + { + m_compiler->m_domTree = domTree; + } + } + + BasicBlock* common = nullptr; + weight_t oldWeight = 0; + BitVecOps::Iter iter(&m_postOrderTraits, sites); + unsigned j; + while (iter.NextElem(&j)) + { + BasicBlock* block = m_dfsTree->GetPostOrder(j); + if (!placementDfs->Contains(block)) + { + common = nullptr; + break; + } + common = common == nullptr ? block : domTree->Intersect(common, block); + oldWeight += block->getBBWeight(m_compiler); + } + + if (common == nullptr) + { + continue; + } + + weight_t commonWeight = common->getBBWeight(m_compiler); + // If the sites merely partition execution, commoning saves no + // dynamic work and can hurt allocation by extending live ranges. + if ((commonWeight >= oldWeight) || + Compiler::fgProfileWeightsEqual(commonWeight, oldWeight, oldWeight * 1e-6)) + { + continue; + } + + rep.ReadBackPlacement = common; + JITDUMP("Planning common readback for %u estimated sites V%02u.[%03u..%03u) -> V%02u in " FMT_BB + " (weight " FMT_WT ", previous total " FMT_WT ")\n", + BitVecOps::Count(&m_postOrderTraits, sites), agg->LclNum, rep.Offset, + rep.Offset + genTypeSize(rep.AccessType), rep.LclNum, common->bbNum, + common->getBBWeight(m_compiler), oldWeight); + } + } +} + //------------------------------------------------------------------------ // StartBlock: -// Handle reaching the end of the currently started block by preparing -// internal state for upcoming basic blocks, and inserting any necessary -// readbacks. +// Reconcile predecessor states and restore readback/writeback status for this block. // // Parameters: // block - The block @@ -1689,8 +1930,7 @@ 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. + // Descriptors are reset between visits; restore this block's pending state below. for (AggregateInfo* agg : m_aggregates) { for (Replacement& rep : agg->Replacements) @@ -1703,82 +1943,150 @@ Statement* ReplaceVisitor::StartBlock(BasicBlock* block) assert(m_numPendingReadBacks == 0); #endif - // OSR locals and parameters may need an initial read back, which we mark - // when we start the initial BB. - if (block != m_compiler->fgFirstBB) - { - return block->firstStmt(); - } - - Statement* lastInsertedStmt = nullptr; - for (AggregateInfo* agg : m_aggregates) { LclVarDsc* dsc = m_compiler->lvaGetDesc(agg->LclNum); - if (!dsc->lvIsParam && !dsc->lvIsOSRLocal) - { - continue; - } - - JITDUMP("Processing fields of %s V%02u in entry BB " FMT_BB "\n", dsc->lvIsParam ? "parameter" : "OSR-local", - agg->LclNum, block->bbNum); - for (size_t i = 0; i < agg->Replacements.size(); i++) { Replacement& rep = agg->Replacements[i]; - ClearNeedsWriteBack(rep); if (!m_liveness->IsReplacementLiveIn(block, agg->LclNum, (unsigned)i)) { - JITDUMP(" V%02u (%s) ignored because it is not live-in to entry BB\n", rep.LclNum, rep.Description); continue; } - if (!dsc->lvIsParam || - !Promotion::MapsToParameterRegister(m_compiler, agg->LclNum, rep.Offset, rep.AccessType)) + bool pending = false; + bool structCurrent = false; + if (block == m_compiler->fgFirstBB) { - SetNeedsReadBack(rep); - JITDUMP(" V%02u (%s) marked as needing read back\n", rep.LclNum, rep.Description); - continue; + pending = dsc->lvIsParam || dsc->lvIsOSRLocal; + structCurrent = pending; } + else if (!BitVecOps::IsMember(&m_postOrderTraits, m_requiresAlreadyReadBackOnEntry, block->bbPostorderNum)) + { + bool hasPred = false; + pending = true; + structCurrent = true; + for (FlowEdge* edge : block->PredEdges()) + { + BasicBlock* pred = edge->getSourceBlock(); + if (!m_dfsTree->Contains(pred)) + { + continue; + } - // Insert read backs of parameters mapping to registers eagerly to - // set the backend up for recognizing these as register accesses. - GenTree* readBack = Promotion::CreateReadBack(m_compiler, agg->LclNum, rep); - Statement* stmt = m_compiler->fgNewStmtFromTree(readBack); - JITDUMP(" V%02u (%s) is read back eagerly because it is a register parameter\n", rep.LclNum, - rep.Description); - DISPSTMT(stmt); - if (lastInsertedStmt == nullptr) + assert(BitVecOps::IsMember(&m_postOrderTraits, m_processedBlocks, pred->bbPostorderNum)); + hasPred = true; + pending &= BitVecOps::IsMember(m_readBackTraits, m_pendingReadBacks[pred->bbPostorderNum], + rep.ReadBackIndex); + structCurrent &= BitVecOps::IsMember(m_readBackTraits, m_currentStructFields[pred->bbPostorderNum], + rep.ReadBackIndex); + } + pending &= hasPred; + structCurrent &= hasPred; + } + + if (structCurrent) { - m_compiler->fgInsertStmtAtBeg(block, stmt); + ClearNeedsWriteBack(rep); + } + + if (pending) + { + assert(structCurrent); + SetNeedsReadBack(rep); } else { - m_compiler->fgInsertStmtAfter(block, lastInsertedStmt, stmt); + // At a mixed join, read back only on predecessors whose struct is + // current. Loading unconditionally here could read stale fields + // from paths that have updated the replacement instead. + for (FlowEdge* edge : block->PredEdges()) + { + BasicBlock* pred = edge->getSourceBlock(); + if (!m_dfsTree->Contains(pred)) + { + continue; + } + + if (BitVecOps::IsMember(&m_postOrderTraits, m_processedBlocks, pred->bbPostorderNum) && + BitVecOps::IsMember(m_readBackTraits, m_pendingReadBacks[pred->bbPostorderNum], + rep.ReadBackIndex)) + { + InsertReadBackAtEnd(pred, agg->LclNum, rep); + BitVecOps::RemoveElemD(m_readBackTraits, m_pendingReadBacks[pred->bbPostorderNum], + rep.ReadBackIndex); + } + } } - lastInsertedStmt = stmt; } } - // Skip all the eager read-backs if any were inserted. - return lastInsertedStmt == nullptr ? block->firstStmt() : lastInsertedStmt->GetNextStmt(); + return block->firstStmt(); +} + +//------------------------------------------------------------------------ +// InsertReadBackAtEnd: +// Materialize a pending replacement before leaving a block. +// +// Parameters: +// block - Block in which the struct contains the current value. +// structLclNum - Struct local. +// rep - Replacement to initialize. +// +void ReplaceVisitor::InsertReadBackAtEnd(BasicBlock* block, unsigned structLclNum, Replacement& rep) +{ + JITDUMP("Reading back V%02u.[%03u..%03u) -> V%02u near the end of " FMT_BB "\n", structLclNum, rep.Offset, + rep.Offset + genTypeSize(rep.AccessType), rep.LclNum, block->bbNum); + + GenTree* readBack = Promotion::CreateReadBack(m_compiler, structLclNum, rep); + Statement* stmt = m_compiler->fgNewStmtFromTree(readBack); + m_compiler->fgInsertStmtNearEnd(block, stmt); +} + +//------------------------------------------------------------------------ +// MustMaterializeReadBacks: +// Check whether pending readbacks must be materialized before leaving a block. +// +// Parameters: +// block - The block to check. +// +// Returns: +// True at loop/EH boundaries requiring current replacement locals. +// +bool ReplaceVisitor::MustMaterializeReadBacks(BasicBlock* block) +{ + if (block->HasPotentialEHSuccs(m_compiler) || + block->KindIs(BBJ_CALLFINALLY, BBJ_EHFINALLYRET, BBJ_EHFILTERRET, BBJ_EHCATCHRET)) + { + return true; + } + + return block->VisitRegularSuccs(m_compiler, [&](BasicBlock* succ) { + return BitVecOps::IsMember(&m_postOrderTraits, m_requiresAlreadyReadBackOnEntry, succ->bbPostorderNum) + ? BasicBlockVisit::Abort + : BasicBlockVisit::Continue; + }) == BasicBlockVisit::Abort; } //------------------------------------------------------------------------ // EndBlock: -// Handle reaching the end of the currently started block by preparing -// internal state for upcoming basic blocks, and inserting any necessary -// readbacks. +// Save readback/writeback status for successors, materializing readbacks at loop/EH boundaries. // // 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. +// Field descriptors are reset between visits; the saved state determines +// which replacements and original fields are current on entry to successors. // void ReplaceVisitor::EndBlock() { + bool materialize = + BitVecOps::IsMember(&m_postOrderTraits, m_requiresReadBackOnExit, m_currentBlock->bbPostorderNum); + + BitVec& pendingReadBacks = m_pendingReadBacks[m_currentBlock->bbPostorderNum]; + pendingReadBacks = BitVecOps::MakeEmpty(m_readBackTraits); + BitVec& currentStructFields = m_currentStructFields[m_currentBlock->bbPostorderNum]; + currentStructFields = BitVecOps::MakeEmpty(m_readBackTraits); + BitVecOps::AddElemD(&m_postOrderTraits, m_processedBlocks, m_currentBlock->bbPostorderNum); + for (AggregateInfo* agg : m_aggregates) { for (size_t i = 0; i < agg->Replacements.size(); i++) @@ -1789,14 +2097,14 @@ void ReplaceVisitor::EndBlock() { if (m_liveness->IsReplacementLiveOut(m_currentBlock, agg->LclNum, (unsigned)i)) { - JITDUMP("Reading back replacement V%02u.[%03u..%03u) -> V%02u near the end of " FMT_BB ":\n", - agg->LclNum, rep.Offset, rep.Offset + genTypeSize(rep.AccessType), rep.LclNum, - m_currentBlock->bbNum); - - GenTree* readBack = Promotion::CreateReadBack(m_compiler, agg->LclNum, rep); - Statement* stmt = m_compiler->fgNewStmtFromTree(readBack); - DISPSTMT(stmt); - m_compiler->fgInsertStmtNearEnd(m_currentBlock, stmt); + if (materialize || (rep.ReadBackPlacement == m_currentBlock)) + { + InsertReadBackAtEnd(m_currentBlock, agg->LclNum, rep); + } + else + { + BitVecOps::AddElemD(m_readBackTraits, pendingReadBacks, rep.ReadBackIndex); + } } else { @@ -1824,6 +2132,13 @@ void ReplaceVisitor::EndBlock() ClearNeedsReadBack(rep); } + if (!rep.NeedsWriteBack) + { + // A readback leaves the original current too. Preserve that fact + // across blocks until a store to the replacement invalidates it. + BitVecOps::AddElemD(m_readBackTraits, currentStructFields, rep.ReadBackIndex); + } + SetNeedsWriteBack(rep); } } @@ -2186,7 +2501,7 @@ void ReplaceVisitor::InsertPreStatementWriteBacks() // to its field local. // // We normally do this before the first use of the field we find, or before -// we transfer control to any successor. This method handles the case of +// control flow requires a materialized value. This method handles the case of // implicit control flow related to EH; when this basic block is in a // try-region (or filter block) and we find a tree that may throw it eagerly // inserts pending readbacks. @@ -2945,11 +3260,20 @@ PhaseStatus Promotion::Run() JITDUMP("Making replacements\n\n"); - // Make all replacements we decided on. - ReplaceVisitor replacer(this, aggregates, &liveness); - for (BasicBlock* bb : m_compiler->Blocks()) + // Make all replacements in reverse postorder so that forward predecessor + // states are available before processing a block. + if (m_compiler->m_dfsTree == nullptr) { - Statement* firstStmt = replacer.StartBlock(bb); + m_compiler->m_dfsTree = m_compiler->fgComputeDfs(); + } + + FlowGraphDfsTree* dfsTree = m_compiler->m_dfsTree; + ReplaceVisitor replacer(this, aggregates, &liveness, dfsTree); + replacer.PrepareReadBacks(); + for (unsigned i = dfsTree->GetPostOrderCount(); i > 0; i--) + { + BasicBlock* bb = dfsTree->GetPostOrder(i - 1); + Statement* firstStmt = replacer.StartBlock(bb); JITDUMP("\nReplacing in "); DBEXEC(m_compiler->verbose, bb->dspBlockHeader()); @@ -3072,17 +3396,15 @@ GenTree* Promotion::EffectiveUser(Compiler::GenTreeStack& ancestors) // expected to map to a register. // // Parameters: -// comp - Compiler instance -// lclNum - Local being accessed into -// offset - Offset being accessed at -// accessType - Type of access -// allowBitwiseExtraction - Whether to allow mappings requiring extraction or a register-class change +// comp - Compiler instance +// lclNum - Local being accessed into +// offset - Offset being accessed at +// accessType - Type of access // // Returns: // True if the access can be efficiently done via a parameter register. // -bool Promotion::MapsToParameterRegister( - Compiler* comp, unsigned lclNum, unsigned offset, var_types accessType, bool allowBitwiseExtraction) +bool Promotion::MapsToParameterRegister(Compiler* comp, unsigned lclNum, unsigned offset, var_types accessType) { assert(lclNum < comp->info.compArgsCount); @@ -3115,12 +3437,6 @@ bool Promotion::MapsToParameterRegister( continue; } - if (!allowBitwiseExtraction && ((offset != seg.Offset) || (genTypeSize(accessType) != seg.Size) || - (varTypeUsesIntReg(accessType) != genIsValidIntReg(seg.GetRegister())))) - { - continue; - } - return true; } diff --git a/src/coreclr/jit/promotion.h b/src/coreclr/jit/promotion.h index e82c0ae436ac9d..f0111d6d87d0c2 100644 --- a/src/coreclr/jit/promotion.h +++ b/src/coreclr/jit/promotion.h @@ -17,15 +17,17 @@ const int PHYSICAL_PROMOTION_MAX_PROMOTIONS_PER_STRUCT = 64; // Represents a single replacement of a (field) access into a struct local. struct Replacement { - unsigned Offset; - var_types AccessType; - unsigned LclNum = BAD_VAR_NUM; + BasicBlock* ReadBackPlacement = nullptr; + unsigned Offset; + var_types AccessType; + unsigned LclNum = BAD_VAR_NUM; + // Dense index into the inter-block pending-readback sets. + unsigned ReadBackIndex = BAD_VAR_NUM; // Is the replacement local (given by LclNum) fresher than the value in the struct local? bool NeedsWriteBack = true; // 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 - // back before transferring control if necessary. + // This may remain true across blocks when all incoming paths agree that + // the struct local contains the current value. bool NeedsReadBack = false; #ifdef DEBUG const char* Description = ""; @@ -154,8 +156,7 @@ class Promotion static bool IsCandidateForPhysicalPromotion(LclVarDsc* dsc); static GenTree* EffectiveUser(Compiler::GenTreeStack& ancestors); - static bool MapsToParameterRegister( - Compiler* comp, unsigned lclNum, unsigned offs, var_types accessType, bool allowBitwiseExtraction = true); + static bool MapsToParameterRegister(Compiler* comp, unsigned lclNum, unsigned offs, var_types accessType); public: explicit Promotion(Compiler* compiler) : m_compiler(compiler) @@ -214,6 +215,8 @@ class PromotionLiveness } void Run(); + bool IsReplacementUsed(BasicBlock* bb, unsigned structLcl, unsigned replacement); + bool IsReplacementDefined(BasicBlock* bb, unsigned structLcl, unsigned replacement); bool IsReplacementLiveIn(BasicBlock* bb, unsigned structLcl, unsigned replacement); bool IsReplacementLiveOut(BasicBlock* bb, unsigned structLcl, unsigned replacement); StructDeaths GetDeathsForStructLocal(GenTreeLclVarCommon* use); @@ -249,6 +252,15 @@ class ReplaceVisitor : public GenTreeVisitor Statement* m_currentStmt = nullptr; BasicBlock* m_currentBlock = nullptr; + FlowGraphDfsTree* m_dfsTree; + BitVecTraits* m_readBackTraits; + BitVecTraits m_postOrderTraits; + BitVec* m_pendingReadBacks; + BitVec* m_currentStructFields; + BitVec m_processedBlocks; + BitVec m_requiresAlreadyReadBackOnEntry; + BitVec m_requiresReadBackOnExit; + public: enum { @@ -257,13 +269,12 @@ 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, + FlowGraphDfsTree* dfsTree); + + void PrepareReadBacks(); bool MadeChanges() { @@ -282,6 +293,10 @@ class ReplaceVisitor : public GenTreeVisitor fgWalkResult PostOrderVisit(GenTree** use, GenTree* user); private: + void PlanReadBacks(); + bool MustMaterializeReadBacks(BasicBlock* block); + void InsertReadBackAtEnd(BasicBlock* block, unsigned structLclNum, Replacement& rep); + void SetNeedsWriteBack(Replacement& rep); void ClearNeedsWriteBack(Replacement& rep); void SetNeedsReadBack(Replacement& rep); diff --git a/src/coreclr/jit/promotionliveness.cpp b/src/coreclr/jit/promotionliveness.cpp index e536884a1e9cc1..c3632d633a9260 100644 --- a/src/coreclr/jit/promotionliveness.cpp +++ b/src/coreclr/jit/promotionliveness.cpp @@ -646,6 +646,42 @@ void PromotionLiveness::FillInLiveness(BitVec& life, BitVec volatileVars, Statem } } +//------------------------------------------------------------------------ +// IsReplacementUsed: +// Check if a replacement field is used before being defined in a block. +// +// Parameters: +// bb - The block +// structLcl - The struct (base) local +// replacementIndex - Index of the replacement +// +// Returns: +// True if the field is in the upward-exposed use set. +// +bool PromotionLiveness::IsReplacementUsed(BasicBlock* bb, unsigned structLcl, unsigned replacementIndex) +{ + unsigned index = m_structLclToTrackedIndex[structLcl] + 1 + replacementIndex; + return BitVecOps::IsMember(m_bvTraits, m_bbInfo[bb->bbNum].VarUse, index); +} + +//------------------------------------------------------------------------ +// IsReplacementDefined: +// Check if a replacement field is fully defined in a block. +// +// Parameters: +// bb - The block +// structLcl - The struct (base) local +// replacementIndex - Index of the replacement +// +// Returns: +// True if the field is in the definition set. +// +bool PromotionLiveness::IsReplacementDefined(BasicBlock* bb, unsigned structLcl, unsigned replacementIndex) +{ + unsigned index = m_structLclToTrackedIndex[structLcl] + 1 + replacementIndex; + return BitVecOps::IsMember(m_bvTraits, m_bbInfo[bb->bbNum].VarDef, index); +} + //------------------------------------------------------------------------ // IsReplacementLiveIn: // Check if a replacement field is live at the start of a basic block. diff --git a/src/tests/JIT/Directed/physicalpromotion/physicalpromotion.cs b/src/tests/JIT/Directed/physicalpromotion/physicalpromotion.cs index 57c6049ca24979..e8e914cc4e002a 100644 --- a/src/tests/JIT/Directed/physicalpromotion/physicalpromotion.cs +++ b/src/tests/JIT/Directed/physicalpromotion/physicalpromotion.cs @@ -104,6 +104,126 @@ public static void Overlappy() Consume(lcl3); } + [Theory] + [InlineData(0, 0U)] + [InlineData(1, 60U)] + [InlineData(2, 120U)] + [InlineData(3, 54U)] + [InlineData(4, 49U)] + [InlineData(5, 70U)] + [InlineData(6, 0x56781245U)] + [InlineData(7, 46U)] + public static void ReadbacksAcrossBranches(int path, uint expected) + { + S value = new S { A = 17, B = 29 }; + Assert.Equal(expected, path is 6 ? ReadbackAfterPartialWrite(value) : ReadbacksAcrossBranchesCore(value, path)); + Assert.Equal(path is 0 ? 0U : path is 1 ? 1U : 46U, ReadbackOnlyOnSelectedPath(value, path)); + Assert.Equal(path switch { 0 => 29U, 1 => 63U, 2 => 80U, 3 => 70U, 4 => 120U, _ => 46U }, + ReadbacksAtLiveJoin(value, path)); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static uint ReadbacksAtLiveJoin(S value, int path) + { + if (path == 0) + { + return value.B; + } + + uint result = 0; + if (path == 1) + { + result = value.A; + } + else if (path == 2) + { + result = value.A * 2; + } + else if (path == 3) + { + value.A = 41; + } + else if (path == 4) + { + value = GetReadbackValue(); + } + + return result + value.A + value.B; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static uint ReadbackAfterPartialWrite(S value) + { + value.C = 0x12345678; + return value.A + value.B; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static uint ReadbackOnlyOnSelectedPath(S value, int path) + { + if (path == 0) + { + return 0; + } + + if (path == 1) + { + return 1; + } + + return value.A + value.B; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static uint ReadbacksAcrossBranchesCore(S value, int path) + { + if (path == 0) + { + return 0; + } + + Consume(value); + switch (path) + { + case 1: + value.A = 31; + break; + case 2: + value = GetReadbackValue(); + break; + case 3: + value.B = 37; + break; + case 4: + for (int i = 0; i < 3; i++) + { + value.A += (uint)i; + Consume(value); + } + break; + case 5: + try + { + value.A = 41; + ThrowForReadback(); + } + catch (InvalidOperationException) + { + Consume(value); + } + break; + } + + Consume(value); + return value.A + value.B; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static S GetReadbackValue() => new S { A = 73, B = 47 }; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ThrowForReadback() => throw new InvalidOperationException(); + [MethodImpl(MethodImplOptions.NoInlining)] private static void Consume(T val) {