diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index df7e672d955ea1..a25a04787c5fa6 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4716,14 +4716,14 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // DoPhase(this, PHASE_EARLY_LIVENESS, &Compiler::fgEarlyLiveness); - // Promote struct locals based on primitive access patterns - // - DoPhase(this, PHASE_PHYSICAL_PROMOTION, &Compiler::PhysicalPromotion); - // Run a simple forward substitution pass. // DoPhase(this, PHASE_FWD_SUB, &Compiler::fgForwardSub); + // Promote struct locals based on primitive access patterns + // + DoPhase(this, PHASE_PHYSICAL_PROMOTION, &Compiler::PhysicalPromotion); + // Locals tree list is no longer kept valid. fgNodeThreading = NodeThreading::None; diff --git a/src/coreclr/jit/promotion.cpp b/src/coreclr/jit/promotion.cpp index f2f32daa9d76c8..caacd8c99c24a4 100644 --- a/src/coreclr/jit/promotion.cpp +++ b/src/coreclr/jit/promotion.cpp @@ -1358,6 +1358,7 @@ void ReplaceVisitor::LoadStoreAroundCall(GenTreeCall* call, GenTree* user) if ((m_aggregates[argNodeLcl->GetLclNum()] != nullptr) && IsPromotedStructLocalDying(argNodeLcl)) { argNodeLcl->gtFlags |= GTF_VAR_DEATH; + CheckForwardSubForLastUse(argNodeLcl->GetLclNum()); } } } @@ -1498,7 +1499,11 @@ void ReplaceVisitor::ReplaceLocal(GenTree** use, GenTree* user) *use = m_compiler->gtNewLclvNode(rep.LclNum, accessType); } - (*use)->gtFlags |= lcl->gtFlags & GTF_VAR_DEATH; + if ((lcl->gtFlags & GTF_VAR_DEATH) != 0) + { + (*use)->gtFlags |= GTF_VAR_DEATH; + CheckForwardSubForLastUse(rep.LclNum); + } if (isDef) { @@ -1540,6 +1545,30 @@ void ReplaceVisitor::ReplaceLocal(GenTree** use, GenTree* user) m_madeChanges = true; } +//------------------------------------------------------------------------ +// CheckForwardSubForLastUse: +// Indicate that a local has a last use in the current statement and that +// there thus may be a forward substitution opportunity. +// +// Parameters: +// lclNum - The local number with a last use in this statement. +// +void ReplaceVisitor::CheckForwardSubForLastUse(unsigned lclNum) +{ + if (m_currentBlock->firstStmt() == m_currentStmt) + { + return; + } + + Statement* prevStmt = m_currentStmt->GetPrevStmt(); + GenTree* prevNode = prevStmt->GetRootNode(); + + if (prevNode->OperIsLocalStore() && (prevNode->AsLclVarCommon()->GetLclNum() == lclNum)) + { + m_mayHaveForwardSub = true; + } +} + //------------------------------------------------------------------------ // StoreBeforeReturn: // Handle a return of a potential struct local. @@ -1780,7 +1809,7 @@ PhaseStatus Promotion::Run() for (Statement* stmt : bb->Statements()) { DISPSTMT(stmt); - replacer.StartStatement(); + replacer.StartStatement(stmt); replacer.WalkTree(stmt->GetRootNodePointer(), nullptr); if (replacer.MadeChanges()) @@ -1790,6 +1819,15 @@ PhaseStatus Promotion::Run() JITDUMP("New statement:\n"); DISPSTMT(stmt); } + + if (replacer.MayHaveForwardSubOpportunity()) + { + JITDUMP("Invoking forward sub due to a potential opportunity\n"); + while ((stmt != bb->firstStmt()) && m_compiler->fgForwardSubStatement(stmt->GetPrevStmt())) + { + m_compiler->fgRemoveStmt(bb, stmt->GetPrevStmt()); + } + } } replacer.EndBlock(); diff --git a/src/coreclr/jit/promotion.h b/src/coreclr/jit/promotion.h index 24ebffe722e7ea..c0b65bef76a3ef 100644 --- a/src/coreclr/jit/promotion.h +++ b/src/coreclr/jit/promotion.h @@ -248,10 +248,14 @@ class DecompositionPlan; class ReplaceVisitor : public GenTreeVisitor { + friend class DecompositionPlan; + jitstd::vector& m_aggregates; PromotionLiveness* m_liveness; bool m_madeChanges = false; bool m_hasPendingReadBacks = false; + bool m_mayHaveForwardSub = false; + Statement* m_currentStmt = nullptr; BasicBlock* m_currentBlock = nullptr; public: @@ -271,6 +275,11 @@ class ReplaceVisitor : public GenTreeVisitor return m_madeChanges; } + bool MayHaveForwardSubOpportunity() + { + return m_mayHaveForwardSub; + } + void StartBlock(BasicBlock* block) { m_currentBlock = block; @@ -278,9 +287,11 @@ class ReplaceVisitor : public GenTreeVisitor void EndBlock(); - void StartStatement() + void StartStatement(Statement* stmt) { - m_madeChanges = false; + m_currentStmt = stmt; + m_madeChanges = false; + m_mayHaveForwardSub = false; } fgWalkResult PostOrderVisit(GenTree** use, GenTree* user); @@ -290,6 +301,7 @@ class ReplaceVisitor : public GenTreeVisitor void LoadStoreAroundCall(GenTreeCall* call, GenTree* user); bool IsPromotedStructLocalDying(GenTreeLclVarCommon* structLcl); void ReplaceLocal(GenTree** use, GenTree* user); + void CheckForwardSubForLastUse(unsigned lclNum); void StoreBeforeReturn(GenTreeUnOp* ret); void WriteBackBefore(GenTree** use, unsigned lcl, unsigned offs, unsigned size); bool MarkForReadBack(unsigned lcl, unsigned offs, unsigned size); diff --git a/src/coreclr/jit/promotiondecomposition.cpp b/src/coreclr/jit/promotiondecomposition.cpp index d0df8b9e17fa62..af70d8145956cb 100644 --- a/src/coreclr/jit/promotiondecomposition.cpp +++ b/src/coreclr/jit/promotiondecomposition.cpp @@ -50,6 +50,7 @@ class DecompositionPlan }; Compiler* m_compiler; + ReplaceVisitor* m_replacer; jitstd::vector& m_aggregates; PromotionLiveness* m_liveness; GenTree* m_store; @@ -61,6 +62,7 @@ class DecompositionPlan public: DecompositionPlan(Compiler* comp, + ReplaceVisitor* replacer, jitstd::vector& aggregates, PromotionLiveness* liveness, GenTree* store, @@ -68,6 +70,7 @@ class DecompositionPlan bool dstInvolvesReplacements, bool srcInvolvesReplacements) : m_compiler(comp) + , m_replacer(replacer) , m_aggregates(aggregates) , m_liveness(liveness) , m_store(store) @@ -731,6 +734,7 @@ class DecompositionPlan if (srcDeaths.IsReplacementDying((unsigned)replacementIndex)) { src->gtFlags |= GTF_VAR_DEATH; + m_replacer->CheckForwardSubForLastUse(entry.FromLclNum); } } } @@ -1009,7 +1013,7 @@ void ReplaceVisitor::HandleStore(GenTree** use, GenTree* user) DecompositionStatementList result; EliminateCommasInBlockOp(store, &result); - DecompositionPlan plan(m_compiler, m_aggregates, m_liveness, store, src, dstInvolvesReplacements, + DecompositionPlan plan(m_compiler, this, m_aggregates, m_liveness, store, src, dstInvolvesReplacements, srcInvolvesReplacements); if (dstInvolvesReplacements)