From 269feef89fb15c1031b7056e34738935d562c607 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 21 Jul 2023 23:12:21 +0200 Subject: [PATCH 1/2] JIT: Optimize liveness fixpoint computation When propagating liveness as part of the fixpoint computation we union the live-in state of all successors into the live-out state. We then further handle EH successors more conservatively since control can flow to those from any point in the BB. The all-successors include successors from two additional sources compared to the regular successors: 1. They include the EH successors, but we already handle these conservatively later, so this is unnecessary. 2. They include EH successors of regular successors, but this liveness is already contributed from the regular successors since they will contain it in their live-in state at some point during the fixpoint computation. Thus we can switch the first loop in liveness to only look at regular successors. --- src/coreclr/jit/block.h | 3 + src/coreclr/jit/compiler.hpp | 89 +++++++++++++++++++++++++++ src/coreclr/jit/liveness.cpp | 7 ++- src/coreclr/jit/promotionliveness.cpp | 2 +- 4 files changed, 98 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index cbdcd66af8590f..469dd89e7a0465 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -1227,6 +1227,9 @@ struct BasicBlock : private LIR::Range template BasicBlockVisit VisitAllSuccs(Compiler* comp, TFunc func); + template + BasicBlockVisit VisitRegularSuccs(Compiler* comp, TFunc func); + bool HasPotentialEHSuccs(Compiler* comp); // BBSuccList: adapter class for forward iteration of block successors, using range-based `for`, diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index ae9e180dbe83df..07c68148c30700 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -651,6 +651,95 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func) return BasicBlockVisit::Continue; } +//------------------------------------------------------------------------------ +// VisitRegularSuccs: Visit regular successors of this block. +// +// Arguments: +// comp - Compiler instance +// func - Callback +// +// Returns: +// Whether or not the visiting was aborted. +// +template +BasicBlockVisit BasicBlock::VisitRegularSuccs(Compiler* comp, TFunc func) +{ + switch (bbJumpKind) + { + case BBJ_EHFILTERRET: + RETURN_ON_ABORT(func(bbJumpDest)); + break; + + case BBJ_EHFINALLYRET: + { + EHblkDsc* ehDsc = comp->ehGetDsc(getHndIndex()); + assert(ehDsc->HasFinallyHandler()); + + BasicBlock* begBlk; + BasicBlock* endBlk; + comp->ehGetCallFinallyBlockRange(getHndIndex(), &begBlk, &endBlk); + + BasicBlock* finBeg = ehDsc->ebdHndBeg; + + for (BasicBlock* bcall = begBlk; bcall != endBlk; bcall = bcall->bbNext) + { + if ((bcall->bbJumpKind != BBJ_CALLFINALLY) || (bcall->bbJumpDest != finBeg)) + { + continue; + } + + assert(bcall->isBBCallAlwaysPair()); + + RETURN_ON_ABORT(func(bcall->bbNext)); + } + + break; + } + + case BBJ_CALLFINALLY: + case BBJ_EHCATCHRET: + case BBJ_LEAVE: + case BBJ_ALWAYS: + RETURN_ON_ABORT(func(bbJumpDest)); + break; + + case BBJ_NONE: + RETURN_ON_ABORT(func(bbNext)); + break; + + case BBJ_COND: + RETURN_ON_ABORT(func(bbNext)); + + if (bbJumpDest != bbNext) + { + RETURN_ON_ABORT(func(bbJumpDest)); + } + + break; + + case BBJ_SWITCH: + { + Compiler::SwitchUniqueSuccSet sd = comp->GetDescriptorForSwitch(this); + for (unsigned i = 0; i < sd.numDistinctSuccs; i++) + { + RETURN_ON_ABORT(func(sd.nonDuplicates[i])); + } + + break; + } + + case BBJ_THROW: + case BBJ_RETURN: + case BBJ_EHFAULTRET: + break; + + default: + unreached(); + } + + return BasicBlockVisit::Continue; +} + #undef RETURN_ON_ABORT //------------------------------------------------------------------------------ diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 517dc848dc6b35..6174d730766268 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -1190,8 +1190,11 @@ class LiveVarAnalysis } } - // Additionally, union in all the live-in tracked vars of successors. - block->VisitAllSuccs(m_compiler, [=](BasicBlock* succ) { + // Additionally, union in all the live-in tracked vars of regular + // successors. EH successors need to be handled more conservatively + // (their live-in state is live in this entire basic block). Those are + // handled below. + block->VisitRegularSuccs(m_compiler, [=](BasicBlock* succ) { VarSetOps::UnionD(m_compiler, m_liveOut, succ->bbLiveIn); m_memoryLiveOut |= succ->bbMemoryLiveIn; if (succ->bbNum <= block->bbNum) diff --git a/src/coreclr/jit/promotionliveness.cpp b/src/coreclr/jit/promotionliveness.cpp index 88678ab07c73d0..5f9fffdb95a291 100644 --- a/src/coreclr/jit/promotionliveness.cpp +++ b/src/coreclr/jit/promotionliveness.cpp @@ -344,7 +344,7 @@ bool PromotionLiveness::PerBlockLiveness(BasicBlock* block) BasicBlockLiveness& bbInfo = m_bbInfo[block->bbNum]; BitVecOps::ClearD(m_bvTraits, bbInfo.LiveOut); - block->VisitAllSuccs(m_compiler, [=, &bbInfo](BasicBlock* succ) { + block->VisitRegularSuccs(m_compiler, [=, &bbInfo](BasicBlock* succ) { BitVecOps::UnionD(m_bvTraits, bbInfo.LiveOut, m_bbInfo[succ->bbNum].LiveIn); m_hasPossibleBackEdge |= succ->bbNum <= block->bbNum; return BasicBlockVisit::Continue; From 3c0d832156024c5184bb31bae0119d19948b0985 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 25 Aug 2023 12:38:35 +0200 Subject: [PATCH 2/2] Take handler memory liveness into account --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/liveness.cpp | 24 +++++++++++++++--------- src/coreclr/jit/lsra.cpp | 5 +++-- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index d45284fc463b52..b669795d73623c 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4908,7 +4908,7 @@ class Compiler void fgPerNodeLocalVarLiveness(GenTreeHWIntrinsic* hwintrinsic); #endif // FEATURE_HW_INTRINSICS - void fgAddHandlerLiveVars(BasicBlock* block, VARSET_TP& ehHandlerLiveVars); + void fgAddHandlerLiveVars(BasicBlock* block, VARSET_TP& ehHandlerLiveVars, MemoryKindSet& memoryLiveness); void fgLiveVarAnalysis(bool updateInternalOnly = false); diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 6174d730766268..62f0e1784a1d37 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -1046,6 +1046,7 @@ void Compiler::fgExtendDbgLifetimes() // block - the block in question // ehHandlerLiveVars - On entry, contains an allocated VARSET_TP that the // function will add handler live vars into. +// memoryLiveness - Set of memory liveness that will be added to. // // Notes: // Assumes caller has screened candidate blocks to only those with @@ -1073,7 +1074,7 @@ void Compiler::fgExtendDbgLifetimes() // Console.WriteLine("In catch 1"); // } // -void Compiler::fgAddHandlerLiveVars(BasicBlock* block, VARSET_TP& ehHandlerLiveVars) +void Compiler::fgAddHandlerLiveVars(BasicBlock* block, VARSET_TP& ehHandlerLiveVars, MemoryKindSet& memoryLiveness) { assert(block->HasPotentialEHSuccs(this)); @@ -1087,6 +1088,7 @@ void Compiler::fgAddHandlerLiveVars(BasicBlock* block, VARSET_TP& ehHandlerLiveV if (HBtab->HasFilter()) { VarSetOps::UnionD(this, ehHandlerLiveVars, HBtab->ebdFilter->bbLiveIn); + memoryLiveness |= HBtab->ebdFilter->bbMemoryLiveIn; #if defined(FEATURE_EH_FUNCLETS) // The EH subsystem can trigger a stack walk after the filter // has returned, but before invoking the handler, and the only @@ -1095,11 +1097,13 @@ void Compiler::fgAddHandlerLiveVars(BasicBlock* block, VARSET_TP& ehHandlerLiveV // must report as live any variables live-out of the filter // (which is the same as those live-in to the handler) VarSetOps::UnionD(this, ehHandlerLiveVars, HBtab->ebdHndBeg->bbLiveIn); + memoryLiveness |= HBtab->ebdHndBeg->bbMemoryLiveIn; #endif // FEATURE_EH_FUNCLETS } else { VarSetOps::UnionD(this, ehHandlerLiveVars, HBtab->ebdHndBeg->bbLiveIn); + memoryLiveness |= HBtab->ebdHndBeg->bbMemoryLiveIn; } /* If we have nested try's edbEnclosing will provide them */ @@ -1118,8 +1122,9 @@ void Compiler::fgAddHandlerLiveVars(BasicBlock* block, VARSET_TP& ehHandlerLiveV if (bbInFilterBBRange(block)) { - block->VisitEHSecondPassSuccs(this, [this, &ehHandlerLiveVars](BasicBlock* succ) { + block->VisitEHSecondPassSuccs(this, [this, &ehHandlerLiveVars, &memoryLiveness](BasicBlock* succ) { VarSetOps::UnionD(this, ehHandlerLiveVars, succ->bbLiveIn); + memoryLiveness |= succ->bbMemoryLiveIn; return BasicBlockVisit::Continue; }); } @@ -1217,16 +1222,12 @@ class LiveVarAnalysis /* Compute the 'm_liveIn' set */ VarSetOps::LivenessD(m_compiler, m_liveIn, block->bbVarDef, block->bbVarUse, m_liveOut); - // Even if block->bbMemoryDef is set, we must assume that it doesn't kill memory liveness from m_memoryLiveOut, - // since (without proof otherwise) the use and def may touch different memory at run-time. - m_memoryLiveIn = m_memoryLiveOut | block->bbMemoryUse; - // Does this block have implicit exception flow to a filter or handler? // If so, include the effects of that flow. if (block->HasPotentialEHSuccs(m_compiler)) { VarSetOps::ClearD(m_compiler, m_ehHandlerLiveVars); - m_compiler->fgAddHandlerLiveVars(block, m_ehHandlerLiveVars); + m_compiler->fgAddHandlerLiveVars(block, m_ehHandlerLiveVars, m_memoryLiveOut); VarSetOps::UnionD(m_compiler, m_liveIn, m_ehHandlerLiveVars); VarSetOps::UnionD(m_compiler, m_liveOut, m_ehHandlerLiveVars); @@ -1235,7 +1236,11 @@ class LiveVarAnalysis m_hasPossibleBackEdge = true; } - /* Has there been any change in either live set? */ + // Even if block->bbMemoryDef is set, we must assume that it doesn't kill memory liveness from m_memoryLiveOut, + // since (without proof otherwise) the use and def may touch different memory at run-time. + m_memoryLiveIn = m_memoryLiveOut | block->bbMemoryUse; + + // Has there been any change in either live set? bool liveInChanged = !VarSetOps::Equal(m_compiler, block->bbLiveIn, m_liveIn); if (liveInChanged || !VarSetOps::Equal(m_compiler, block->bbLiveOut, m_liveOut)) @@ -2528,7 +2533,8 @@ void Compiler::fgInterBlockLocalVarLiveness() if (block->HasPotentialEHSuccs(this)) { - fgAddHandlerLiveVars(block, volatileVars); + MemoryKindSet memoryLiveness = 0; + fgAddHandlerLiveVars(block, volatileVars, memoryLiveness); // volatileVars is a subset of exceptVars noway_assert(VarSetOps::IsSubset(this, volatileVars, exceptVars)); diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 5c8fe4aae77889..16951d4f686966 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -2400,8 +2400,9 @@ void LinearScan::checkLastUses(BasicBlock* block) // We may have exception vars in the liveIn set of exception blocks that are not computed live. if (block->HasPotentialEHSuccs(compiler)) { - VARSET_TP ehHandlerLiveVars(VarSetOps::MakeEmpty(compiler)); - compiler->fgAddHandlerLiveVars(block, ehHandlerLiveVars); + VARSET_TP ehHandlerLiveVars(VarSetOps::MakeEmpty(compiler)); + MemoryKindSet memoryLiveness = emptyMemoryKindSet; + compiler->fgAddHandlerLiveVars(block, ehHandlerLiveVars, memoryLiveness); VarSetOps::DiffD(compiler, liveInNotComputedLive, ehHandlerLiveVars); } VarSetOps::Iter liveInNotComputedLiveIter(compiler, liveInNotComputedLive);