diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index de116a31f9ebb1..eae466e33b430f 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -72,22 +72,24 @@ inline var_types genActualType(T value); * Forward declarations */ -struct InfoHdr; // defined in GCInfo.h -struct escapeMapping_t; // defined in fgdiagnostic.cpp -class emitter; // defined in emit.h -struct ShadowParamVarInfo; // defined in GSChecks.cpp -struct InitVarDscInfo; // defined in registerargconvention.h -class FgStack; // defined in fgbasic.cpp -class Instrumentor; // defined in fgprofile.cpp -class SpanningTreeVisitor; // defined in fgprofile.cpp -class CSE_DataFlow; // defined in optcse.cpp -struct CSEdsc; // defined in optcse.h -class CSE_HeuristicCommon; // defined in optcse.h -class OptBoolsDsc; // defined in optimizer.cpp -struct JumpThreadInfo; // defined in redundantbranchopts.cpp -class ProfileSynthesis; // defined in profilesynthesis.h -class PerLoopInfo; // defined in inductionvariableopts.cpp -class RangeCheck; // defined in rangecheck.h +struct InfoHdr; // defined in GCInfo.h +struct escapeMapping_t; // defined in fgdiagnostic.cpp +class emitter; // defined in emit.h +struct ShadowParamVarInfo; // defined in GSChecks.cpp +struct InitVarDscInfo; // defined in registerargconvention.h +class FgStack; // defined in fgbasic.cpp +class Instrumentor; // defined in fgprofile.cpp +class SpanningTreeVisitor; // defined in fgprofile.cpp +class CSE_DataFlow; // defined in optcse.cpp +struct CSEdsc; // defined in optcse.h +class CSE_HeuristicCommon; // defined in optcse.h +class OptBoolsDsc; // defined in optimizer.cpp +struct JumpThreadInfo; // defined in redundantbranchopts.cpp +class ProfileSynthesis; // defined in profilesynthesis.h +class PerLoopInfo; // defined in inductionvariableopts.cpp +class IncrementalSsaBuilder; // defined in ssabuilder.h +struct UseDefLocation; // defined in ssabuilder.h +class RangeCheck; // defined in rangecheck.h #ifdef TARGET_WASM class WasmInterval; // defined in fgwasm.h enum class WasmValueType : unsigned; @@ -8167,6 +8169,22 @@ class Compiler bool optRemoveUnusedIVs(FlowGraphNaturalLoop* loop, PerLoopInfo* loopLocals); bool optIsUpdateOfIVWithoutSideEffects(GenTree* tree, unsigned lclNum); + bool optReplaceUnenregisterablePrimaryIVs(FlowGraphNaturalLoop* loop, PerLoopInfo* loopLocals); + bool optPrimaryIVStaysEHLiveAfterReplacement(unsigned lclNum, FlowGraphNaturalLoop* loop); + GenTreePhiArg* optGetPrimaryIVEntryArg(FlowGraphNaturalLoop* loop, Statement* headerPhiStmt); + bool optTryReplaceUnenregisterablePrimaryIV(FlowGraphNaturalLoop* loop, + unsigned lclNum, + Statement* headerPhiStmt, + PerLoopInfo* loopLocals); + bool optCanReplaceUnenregisterablePrimaryIV(unsigned lclNum, FlowGraphNaturalLoop* loop); + bool optPrimaryIVCanCrossHandler(unsigned lclNum, EHblkDsc* eh, FlowGraphNaturalLoop* loop); + void optReplaceIVUses(unsigned lclNum, + unsigned newLclNum, + BasicBlock* block, + Statement* stmt, + IncrementalSsaBuilder* ssaBuilder, + ArrayStack* uses); + // Redundant branch opts // enum class JumpThreadCheckResult diff --git a/src/coreclr/jit/inductionvariableopts.cpp b/src/coreclr/jit/inductionvariableopts.cpp index 3384175eee12ae..54cbc8bb41abb2 100644 --- a/src/coreclr/jit/inductionvariableopts.cpp +++ b/src/coreclr/jit/inductionvariableopts.cpp @@ -55,6 +55,7 @@ #include "jitpch.h" #include "scev.h" +#include "ssabuilder.h" // Data structure that keeps track of per-loop info, like occurrences and suspension-points inside loops. class PerLoopInfo @@ -895,8 +896,10 @@ bool Compiler::optWidenPrimaryIV(FlowGraphNaturalLoop* loop, unsigned lclNum, Sc } // If the IV is not enregisterable, or if it lives into a handler, then - // uses/defs are going to go to stack regardless. - if (lclDsc->lvDoNotEnregister || lclDsc->IsLiveInOutOfHandler()) + // uses/defs are going to go to stack regardless. Note that the + // live-in/out-of-handler state is only meaningful for tracked locals; newly + // introduced SSA IVs (e.g. from enregisterable IV replacement) are untracked. + if (lclDsc->lvDoNotEnregister || (lclDsc->lvTracked && lclDsc->IsLiveInOutOfHandler())) { JITDUMP(" V%02u is marked DNER or lives into a handler\n", lclNum); return false; @@ -1028,6 +1031,563 @@ bool Compiler::optWidenPrimaryIV(FlowGraphNaturalLoop* loop, unsigned lclNum, Sc return true; } +//------------------------------------------------------------------------ +// optPrimaryIVStaysEHLiveAfterReplacement: Check whether a fresh local that +// replaces the in-loop occurrences of a primary IV would itself be marked live +// in/out of a handler. +// +// Parameters: +// lclNum - The primary IV (must be a tracked local) +// loop - The loop +// +// Returns: +// True if the replacement local would still be live in/out of a handler. +// +// Remarks: +// The fresh local inherits the IV's liveness inside the loop. It is therefore +// marked live in/out of a handler if the IV is live across an EH region +// boundary that is contained in the loop (i.e. the EH-liveness comes from a +// handler inside the loop rather than one enclosing it). In that case the +// replacement is pointless since the fresh local cannot be enregistered +// either. +// +bool Compiler::optPrimaryIVStaysEHLiveAfterReplacement(unsigned lclNum, FlowGraphNaturalLoop* loop) +{ + LclVarDsc* dsc = lvaGetDesc(lclNum); + assert(dsc->lvTracked); + unsigned varIndex = dsc->lvVarIndex; + + BasicBlockVisit result = loop->VisitLoopBlocks([=](BasicBlock* block) { + if (block->hasEHBoundaryIn() && VarSetOps::IsMember(this, block->bbLiveIn, varIndex)) + { + return BasicBlockVisit::Abort; + } + + if (block->hasEHBoundaryOut() && VarSetOps::IsMember(this, block->bbLiveOut, varIndex)) + { + return BasicBlockVisit::Abort; + } + + return BasicBlockVisit::Continue; + }); + + return result == BasicBlockVisit::Abort; +} + +//------------------------------------------------------------------------ +// optReplaceUnenregisterablePrimaryIVs: Look for primary IVs that cannot be +// enregistered (because they are marked DNER or live in/out of a handler) and +// try to replace them with a fresh enregisterable local inside the loop. +// +// Parameters: +// loop - The loop +// loopInfo - Data structure for tracking loop info, like local occurrences +// +// Returns: +// True if any primary IV was replaced. +// +bool Compiler::optReplaceUnenregisterablePrimaryIVs(FlowGraphNaturalLoop* loop, PerLoopInfo* loopInfo) +{ + JITDUMP("Considering primary IVs of " FMT_LP " for enregisterable replacement\n", loop->GetIndex()); + + // Collect the candidate header phis first. Replacing an IV deletes its + // header phi, which would otherwise invalidate the iteration below. + ArrayStack candidates(getAllocator(CMK_LoopOpt)); + for (Statement* stmt : loop->GetHeader()->Statements()) + { + if (!stmt->IsPhiDefnStmt()) + { + break; + } + + unsigned lclNum = stmt->GetRootNode()->AsLclVarCommon()->GetLclNum(); + LclVarDsc* lclDsc = lvaGetDesc(lclNum); + + if (!lclDsc->lvTracked) + { + continue; + } + + // We are only interested in IVs that cannot be enregistered (either + // because they are explicitly marked DNER, or because they are live + // in/out of a handler). A fresh local that is only live inside the loop + // can be enregistered instead. + if (!lclDsc->lvDoNotEnregister && !lclDsc->IsLiveInOutOfHandler()) + { + continue; + } + + // We require the IV to be a clean SSA local of a scalar type so that a + // fresh enregisterable local can stand in for it. This filters out the + // DNER reasons (address exposure, field accesses, struct/block ops, + // pinning, ...) that a fresh local would not be able to avoid. + if (!lclDsc->lvInSsa || lclDsc->IsAddressExposed() || lclDsc->lvIsStructField || lclDsc->lvPinned || + varTypeIsStruct(lclDsc)) + { + continue; + } + + // The fresh local takes over the IV's liveness inside the loop, so if + // the IV is live across an EH region contained in the loop then the + // fresh local would itself be marked live in/out of a handler and could + // not be enregistered either. In that case there is nothing to gain. + if (optPrimaryIVStaysEHLiveAfterReplacement(lclNum, loop)) + { + JITDUMP(" V%02u is live across an EH region inside " FMT_LP "; replacement would not be enregisterable\n", + lclNum, loop->GetIndex()); + continue; + } + + candidates.Push(stmt); + } + + unsigned numReplaced = 0; + for (int i = 0; i < candidates.Height(); i++) + { + Statement* headerPhiStmt = candidates.Bottom(i); + unsigned lclNum = headerPhiStmt->GetRootNode()->AsLclVarCommon()->GetLclNum(); + if (optTryReplaceUnenregisterablePrimaryIV(loop, lclNum, headerPhiStmt, loopInfo)) + { + numReplaced++; + } + } + + Metrics.EnregisterableIVsCreated += numReplaced; + return numReplaced > 0; +} + +//------------------------------------------------------------------------ +// optGetPrimaryIVEntryArg: Get the entry (non-backedge) phi argument of a +// primary IV's header phi. +// +// Parameters: +// loop - The loop +// headerPhiStmt - The IV's header phi definition statement +// +// Returns: +// The single entry phi argument, or nullptr if there is more than one. +// +GenTreePhiArg* Compiler::optGetPrimaryIVEntryArg(FlowGraphNaturalLoop* loop, Statement* headerPhiStmt) +{ + GenTreePhi* phi = headerPhiStmt->GetRootNode()->AsLclVar()->Data()->AsPhi(); + GenTreePhiArg* entryArg = nullptr; + for (GenTreePhi::Use& use : phi->Uses()) + { + GenTreePhiArg* arg = use.GetNode()->AsPhiArg(); + if (loop->ContainsBlock(arg->gtPredBB)) + { + continue; + } + + if (entryArg != nullptr) + { + // More than one entry edge. + return nullptr; + } + + entryArg = arg; + } + + return entryArg; +} + +//------------------------------------------------------------------------ +// optTryReplaceUnenregisterablePrimaryIV: Try to replace a primary IV that +// cannot be enregistered with a fresh enregisterable local. +// +// Parameters: +// loop - The loop +// lclNum - The primary IV +// headerPhiStmt - The IV's header phi definition statement +// loopInfo - Data structure for tracking loop info, like local occurrences +// +// Returns: +// True if the IV was replaced. +// +// Remarks: +// The new local is created as a proper SSA induction variable (with its own +// header phi) using IncrementalSsaBuilder, so that the subsequent +// scalar-evolution-based optimizations (strength reduction, widening) can +// optimize it. The old IV's header phi is deleted so that those optimizations +// do not analyze it (it no longer has an in-loop store). The original local +// keeps holding the correct value outside the loop: we initialize the new +// local in the preheader and store it back into the original in the loop's +// regular exits where it is live. SSA for the old IV is left invalid, which +// is acceptable since SSA is torn down at the end of this phase. +// +bool Compiler::optTryReplaceUnenregisterablePrimaryIV(FlowGraphNaturalLoop* loop, + unsigned lclNum, + Statement* headerPhiStmt, + PerLoopInfo* loopInfo) +{ + JITDUMP(" V%02u is a primary IV that cannot be enregistered; trying to replace it with an enregisterable local\n", + lclNum); + + // A fresh local would have to be marked DNER if the IV is accessed as a + // field, so there would be nothing to gain. + auto isNotFieldAccess = [](BasicBlock* block, Statement* stmt, GenTreeLclVarCommon* tree) { + return !tree->OperIs(GT_LCL_FLD, GT_STORE_LCL_FLD); + }; + + if (!loopInfo->VisitOccurrences(loop, lclNum, isNotFieldAccess)) + { + JITDUMP(" V%02u is accessed as a field inside the loop; skipping\n", lclNum); + return false; + } + + if (!optCanReplaceUnenregisterablePrimaryIV(lclNum, loop)) + { + return false; + } + + var_types ivType = lvaGetDesc(lclNum)->TypeGet(); + unsigned newLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("Enregisterable replacement for IV V%02u", lclNum))); + lvaGetDesc(newLclNum)->lvType = ivType; + + // Determine the value the IV enters the loop with (and its VN) from the + // entry argument of the header phi. When that value is a constant we + // initialize the new local to the constant directly to avoid a + // store-then-reload round trip. + GenTree* initVal = nullptr; + GenTreePhiArg* entryArg = optGetPrimaryIVEntryArg(loop, headerPhiStmt); + if (entryArg != nullptr) + { + LclSsaVarDsc* entrySsa = lvaGetDesc(lclNum)->GetPerSsaData(entryArg->GetSsaNum()); + GenTree* entryDef = entrySsa->GetDefNode() == nullptr ? nullptr : entrySsa->GetDefNode()->Data(); + if ((entryDef != nullptr) && entryDef->IsIntegralConst()) + { + initVal = gtCloneExpr(entryDef); + } + else + { + // We do not set SSA info on this use of the old local; the old IV + // is being removed from the loop and its SSA is left invalid. + GenTreeLclVar* initUse = gtNewLclvNode(lclNum, ivType); + initUse->gtVNPair = entrySsa->m_vnPair; + initVal = initUse; + } + } + else + { + initVal = gtNewLclvNode(lclNum, ivType); + } + + IncrementalSsaBuilder ssaBuilder(this, newLclNum); + ArrayStack uses(getAllocator(CMK_LoopOpt)); + + // First definition: initialization of the new local in the preheader. + BasicBlock* preheader = loop->EntryEdge(0)->getSourceBlock(); + GenTreeLclVar* initStore = gtNewStoreLclVarNode(newLclNum, initVal); + Statement* initStmt = fgNewStmtFromTree(initStore); + fgInsertStmtNearEnd(preheader, initStmt); + ssaBuilder.InsertDef(UseDefLocation(preheader, initStmt, initStore)); + + JITDUMP(" Initializing replacement V%02u in preheader " FMT_BB "\n", newLclNum, preheader->bbNum); + DISPSTMT(initStmt); + JITDUMP("\n"); + + // Rename all in-loop occurrences of the IV to the new local, collecting the + // new definitions and uses for SSA construction. Phi definitions are not + // included in the occurrences, so the header phi is left untouched here. + JITDUMP(" Replacing V%02u with V%02u inside the loop\n", lclNum, newLclNum); + auto replace = [=, &ssaBuilder, &uses](BasicBlock* block, Statement* stmt) { + optReplaceIVUses(lclNum, newLclNum, block, stmt, &ssaBuilder, &uses); + return true; + }; + loopInfo->VisitStatementsWithOccurrences(loop, lclNum, replace); + + // Delete the old IV's header phi so the scalar-evolution-based optimizations + // below do not analyze the old IV, which no longer has an in-loop store. + JITDUMP(" Removing old header phi " FMT_STMT "\n", headerPhiStmt->GetID()); + fgRemoveStmt(loop->GetHeader(), headerPhiStmt); + + // Store the final value back into the original local in the loop's regular + // exits where it is live, so that uses after the loop see the right value. + loop->VisitRegularExitBlocks([=, &uses](BasicBlock* exit) { + if (!optLocalIsLiveIntoBlock(lclNum, exit)) + { + return BasicBlockVisit::Continue; + } + + GenTreeLclVar* newUse = gtNewLclvNode(newLclNum, ivType); + GenTree* store = gtNewStoreLclVarNode(lclNum, newUse); + Statement* newStmt = fgNewStmtFromTree(store); + JITDUMP(" V%02u is live into exit block " FMT_BB "; storing replacement V%02u back\n", lclNum, exit->bbNum, + newLclNum); + DISPSTMT(newStmt); + fgInsertStmtAtBeg(exit, newStmt); + uses.Emplace(exit, newStmt, newUse); + + return BasicBlockVisit::Continue; + }); + + // Finalize SSA for the new local: register each use against its reaching + // definition, creating the new IV's header phi lazily as needed. + if (ssaBuilder.FinalizeDefs()) + { + for (int i = 0; i < uses.Height(); i++) + { + ssaBuilder.InsertUse(uses.Bottom(i)); + } + } + + loopInfo->Invalidate(loop); + return true; +} + +//------------------------------------------------------------------------ +// optCanReplaceUnenregisterablePrimaryIV: Check whether it is legal to replace +// the in-loop occurrences of a primary IV that is live in/out of a handler +// with a fresh local. +// +// Parameters: +// lclNum - The primary IV +// loop - The loop +// +// Returns: +// True if the replacement is legal. +// +// Remarks: +// The replacement stops updating the original local inside the loop, so any +// handler reachable from the loop via exceptional flow must not observe the +// in-loop value of the IV. This is the case when, for every such handler, +// either: +// - it is a catch/filter handler and the IV is not live into it (a value +// that is live through the handler is not allowed since the handler can +// resume normal execution after the protected region), or +// - it is a finally/fault handler with no uses of the IV (a value that is +// only live through the handler is fine: on the normal path the final +// value is stored in the loop exits before the handler runs, and on the +// exceptional path the handler re-raises so the stale value is never +// observed afterwards). +// Handlers contained in the loop are always fine since their occurrences are +// replaced as well. +// +bool Compiler::optCanReplaceUnenregisterablePrimaryIV(unsigned lclNum, FlowGraphNaturalLoop* loop) +{ + if (compHndBBtabCount == 0) + { + return true; + } + + BitVecTraits ehTraits(compHndBBtabCount, this); + BitVec checkedRegions = BitVecOps::MakeEmpty(&ehTraits); + + BasicBlockVisit result = loop->VisitLoopBlocks([=, &ehTraits, &checkedRegions](BasicBlock* block) { + return block->VisitEHSuccs(this, [=, &ehTraits, &checkedRegions](BasicBlock* ehSucc) { + unsigned ehIndex = ehSucc->getHndIndex(); + if (BitVecOps::TryAddElemD(&ehTraits, checkedRegions, ehIndex) && + !optPrimaryIVCanCrossHandler(lclNum, ehGetDsc(ehIndex), loop)) + { + return BasicBlockVisit::Abort; + } + + return BasicBlockVisit::Continue; + }); + }); + + if (result == BasicBlockVisit::Abort) + { + JITDUMP(" Cannot replace V%02u; it may be observed in a reachable handler\n", lclNum); + return false; + } + + // We must be able to insert the store-back of the final value into every + // regular exit where the IV is live. Blocks that are part of a call-finally + // pair must remain empty, so we cannot sink into them. We also need all + // predecessors of such exits to come from inside the loop, since otherwise + // the store-back would incorrectly overwrite the original local on a + // non-loop path reaching the exit. + BasicBlockVisit exitResult = loop->VisitRegularExitBlocks([=](BasicBlock* exit) { + if (!optLocalIsLiveIntoBlock(lclNum, exit)) + { + return BasicBlockVisit::Continue; + } + + if (exit->isBBCallFinallyPair() || exit->isBBCallFinallyPairTail()) + { + JITDUMP(" Cannot replace V%02u; live regular exit " FMT_BB " is part of a call-finally pair\n", lclNum, + exit->bbNum); + return BasicBlockVisit::Abort; + } + + for (BasicBlock* pred : exit->PredBlocks()) + { + if (!loop->ContainsBlock(pred)) + { + JITDUMP(" Cannot replace V%02u; live regular exit " FMT_BB " of " FMT_LP + " has a non-loop pred " FMT_BB "\n", + lclNum, exit->bbNum, loop->GetIndex(), pred->bbNum); + return BasicBlockVisit::Abort; + } + } + + return BasicBlockVisit::Continue; + }); + + if (exitResult == BasicBlockVisit::Abort) + { + return false; + } + + return true; +} + +//------------------------------------------------------------------------ +// optPrimaryIVCanCrossHandler: Check whether replacing the in-loop occurrences +// of a primary IV is legal with respect to a single reachable handler. +// +// Parameters: +// lclNum - The primary IV +// eh - The EH region whose handler is reachable from the loop +// loop - The loop +// +// Returns: +// True if the replacement is legal with respect to this handler. +// +bool Compiler::optPrimaryIVCanCrossHandler(unsigned lclNum, EHblkDsc* eh, FlowGraphNaturalLoop* loop) +{ + // Handlers inside the loop have their occurrences replaced along with the + // rest of the loop, so they are fine. + if (loop->ContainsBlock(eh->ebdHndBeg)) + { + return true; + } + + if (!eh->HasFinallyOrFaultHandler()) + { + // A catch/filter handler can resume normal execution after the protected + // region, so the IV must not be live into it (this also rejects values + // that are merely live through the handler). + if (eh->HasFilter() && optLocalIsLiveIntoBlock(lclNum, eh->ebdFilter)) + { + return false; + } + + return !optLocalIsLiveIntoBlock(lclNum, eh->ebdHndBeg); + } + + // A finally/fault handler never resumes normal execution after the protected + // region, so it is fine as long as the IV is not actually used inside it. + // bbVarUse captures exactly the upward-exposed uses (and excludes the SSA PHI + // artifacts at the handler entry). + unsigned varIndex = lvaGetDesc(lclNum)->lvVarIndex; + for (BasicBlock *block = eh->ebdHndBeg, *end = eh->ebdHndLast->Next(); block != end; block = block->Next()) + { + if (VarSetOps::IsMember(this, block->bbVarUse, varIndex)) + { + return false; + } + } + + return true; +} + +//------------------------------------------------------------------------ +// optReplaceIVUses: Replace all non-phi occurrences of a local in a statement +// with a new local of the same type, recording the resulting definitions and +// uses for incremental SSA construction. +// +// Parameters: +// lclNum - Local to replace +// newLclNum - Local to replace it with +// block - The block containing the statement +// stmt - The statement to replace uses in +// ssaBuilder - Incremental SSA builder for "newLclNum"; new definitions are +// registered into it +// uses - [in, out] List that new uses are appended to +// +// Remarks: +// Phi definitions are intentionally left alone; the IV's header phi is +// deleted separately by the caller. +// +void Compiler::optReplaceIVUses(unsigned lclNum, + unsigned newLclNum, + BasicBlock* block, + Statement* stmt, + IncrementalSsaBuilder* ssaBuilder, + ArrayStack* uses) +{ + struct ReplaceVisitor : GenTreeVisitor + { + private: + unsigned m_lclNum; + unsigned m_newLclNum; + BasicBlock* m_block; + Statement* m_stmt; + IncrementalSsaBuilder* m_ssaBuilder; + ArrayStack* m_uses; + + public: + bool MadeChanges = false; + + enum + { + DoPreOrder = true, + }; + + ReplaceVisitor(Compiler* comp, + unsigned lclNum, + unsigned newLclNum, + BasicBlock* block, + Statement* stmt, + IncrementalSsaBuilder* ssaBuilder, + ArrayStack* uses) + : GenTreeVisitor(comp) + , m_lclNum(lclNum) + , m_newLclNum(newLclNum) + , m_block(block) + , m_stmt(stmt) + , m_ssaBuilder(ssaBuilder) + , m_uses(uses) + { + } + + fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) + { + GenTree* node = *use; + if (node->OperIs(GT_STORE_LCL_VAR) && (node->AsLclVarCommon()->GetLclNum() == m_lclNum)) + { + // Leave phi definitions alone; the header phi is deleted separately. + if (node->AsLclVar()->Data()->OperIs(GT_PHI)) + { + return fgWalkResult::WALK_CONTINUE; + } + + // Create a fresh store node to avoid carrying over stale data + // (e.g. SSA numbers, value numbers) from the old local's node. + GenTreeLclVar* newStore = m_compiler->gtNewStoreLclVarNode(m_newLclNum, node->AsLclVar()->Data()); + newStore->gtVNPair = node->gtVNPair; + *use = newStore; + m_ssaBuilder->InsertDef(UseDefLocation(m_block, m_stmt, newStore)); + MadeChanges = true; + } + else if (node->OperIs(GT_LCL_VAR) && (node->AsLclVarCommon()->GetLclNum() == m_lclNum)) + { + // Create a fresh use node to avoid carrying over stale data + // (e.g. SSA numbers, value numbers) from the old local's node. + GenTreeLclVar* newUse = m_compiler->gtNewLclvNode(m_newLclNum, node->TypeGet()); + *use = newUse; + m_uses->Emplace(m_block, m_stmt, newUse); + MadeChanges = true; + } + + return fgWalkResult::WALK_CONTINUE; + } + }; + + ReplaceVisitor visitor(this, lclNum, newLclNum, block, stmt, ssaBuilder, uses); + visitor.WalkTree(stmt->GetRootNodePointer(), nullptr); + if (visitor.MadeChanges) + { + gtSetStmtInfo(stmt); + fgSetStmtSeq(stmt); + JITDUMP("New tree:\n"); + DISPTREE(stmt->GetRootNode()); + JITDUMP("\n"); + } +} + //------------------------------------------------------------------------ // optVisitBoundingExitingBlocks: Visit all the exiting BBJ_COND blocks of the // loop that dominate all the loop's backedges. These exiting blocks bound the @@ -2875,6 +3435,15 @@ PhaseStatus Compiler::optInductionVariables() continue; } + // Replace primary IVs that cannot be enregistered with fresh + // enregisterable locals first, so that the new IVs become candidates + // for the scalar-evolution-based optimizations below (strength + // reduction, widening). + if (optReplaceUnenregisterablePrimaryIVs(loop, &loopInfo)) + { + changed = true; + } + StrengthReductionContext strengthReductionContext(this, scevContext, loop, loopInfo); if (strengthReductionContext.TryStrengthReduce()) { diff --git a/src/coreclr/jit/jitmetadatalist.h b/src/coreclr/jit/jitmetadatalist.h index 7fc8865617b9dc..43766fa67ed3e5 100644 --- a/src/coreclr/jit/jitmetadatalist.h +++ b/src/coreclr/jit/jitmetadatalist.h @@ -43,6 +43,7 @@ JITMETADATAMETRIC(LoopAlignmentCandidates, int, 0) JITMETADATAMETRIC(LoopsAligned, int, 0) JITMETADATAMETRIC(LoopsIVWidened, int, 0) JITMETADATAMETRIC(WidenedIVs, int, 0) +JITMETADATAMETRIC(EnregisterableIVsCreated, int, 0) JITMETADATAMETRIC(UnusedIVsRemoved, int, 0) JITMETADATAMETRIC(LoopsMadeDownwardsCounted, int, 0) JITMETADATAMETRIC(LoopsStrengthReduced, int, 0) diff --git a/src/tests/JIT/opt/Loops/PrimaryIVLiveInHandler.cs b/src/tests/JIT/opt/Loops/PrimaryIVLiveInHandler.cs new file mode 100644 index 00000000000000..4c606558b183cb --- /dev/null +++ b/src/tests/JIT/opt/Loops/PrimaryIVLiveInHandler.cs @@ -0,0 +1,227 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Regression tests for the induction variable optimization that replaces a +// primary IV that is live in/out of a handler (and therefore cannot be +// enregistered) with a fresh enregisterable local inside the loop. The +// transformation must preserve the value of the IV outside the loop on every +// path, including exceptional ones. + +using System; +using System.Runtime.CompilerServices; +using Xunit; + +public class PrimaryIVLiveInHandler +{ + [MethodImpl(MethodImplOptions.NoInlining)] + private static void SideEffect() { } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static int Identity(int x) => x; + + private static int[] MakeData(int n) + { + int[] data = new int[n]; + for (int i = 0; i < n; i++) + { + data[i] = i * 2; + } + + return data; + } + + // The loop counter is reused as the return value local and is live across a + // finally that does not use it. This is the pattern from the original + // regression: the IV is live in/out of the handler and the optimization + // should be able to replace it inside the loop while keeping the after-loop + // value correct. + [MethodImpl(MethodImplOptions.NoInlining)] + private static int SumLiveAcrossFinally(int[] data) + { + int j; + int sum = 0; + try + { + for (j = 0; j < data.Length; j++) + { + sum += data[j]; + } + + j = sum; + } + finally + { + SideEffect(); + } + + return j; + } + + // Same shape but the IV's final value (not overwritten) is what is observed + // after the finally, exercising the store-back into the original local in + // the loop exit. + [MethodImpl(MethodImplOptions.NoInlining)] + private static int CountLiveAcrossFinally(int[] data, out int sum) + { + int j; + sum = 0; + try + { + for (j = 0; j < data.Length; j++) + { + sum += data[j]; + } + } + finally + { + SideEffect(); + } + + return j; + } + + // The loop may throw. On the exceptional path the finally runs (without + // using the IV) and the exception propagates, so the after-loop value is + // never observed. On the normal path it must be correct. + [MethodImpl(MethodImplOptions.NoInlining)] + private static int SumWithThrowingLoop(int[] data, int throwAt) + { + int j; + int sum = 0; + try + { + for (j = 0; j < data.Length; j++) + { + if (j == throwAt) + { + throw new InvalidOperationException(); + } + + sum += data[j]; + } + + j = sum; + } + finally + { + SideEffect(); + } + + return j; + } + + // The IV is live into a catch as a pass-through value used after the + // try/catch. The optimization must not replace it (or must keep the value + // correct) since a catch resumes normal execution. + [MethodImpl(MethodImplOptions.NoInlining)] + private static int SumLiveAcrossCatch(int[] data, int throwAt) + { + int j = 0; + int sum = 0; + try + { + for (j = 0; j < data.Length; j++) + { + if (j == throwAt) + { + throw new InvalidOperationException(); + } + + sum += data[j]; + } + + j = sum; + } + catch (InvalidOperationException) + { + // Falls through to use j below. + } + + return j; + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(10)] + [InlineData(1000)] + public static void SumLiveAcrossFinally_ReturnsSum(int n) + { + int[] data = MakeData(n); + int expected = 0; + for (int i = 0; i < n; i++) + { + expected += data[i]; + } + + Assert.Equal(expected, SumLiveAcrossFinally(data)); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(10)] + [InlineData(1000)] + public static void CountLiveAcrossFinally_ReturnsCountAndSum(int n) + { + int[] data = MakeData(n); + int expectedSum = 0; + for (int i = 0; i < n; i++) + { + expectedSum += data[i]; + } + + int count = CountLiveAcrossFinally(data, out int sum); + Assert.Equal(n, count); + Assert.Equal(expectedSum, sum); + } + + [Fact] + public static void SumWithThrowingLoop_NormalPath_ReturnsSum() + { + int[] data = MakeData(1000); + int expected = 0; + for (int i = 0; i < data.Length; i++) + { + expected += data[i]; + } + + // throwAt is out of range, so the loop never throws. + Assert.Equal(expected, SumWithThrowingLoop(data, -1)); + } + + [Fact] + public static void SumWithThrowingLoop_ExceptionalPath_Throws() + { + int[] data = MakeData(1000); + Assert.Throws(() => SumWithThrowingLoop(data, 500)); + } + + [Theory] + [InlineData(-1)] + [InlineData(0)] + [InlineData(500)] + public static void SumLiveAcrossCatch_ReturnsExpected(int throwAt) + { + int[] data = MakeData(1000); + + int expected; + if ((uint)throwAt < (uint)data.Length) + { + // The catch is taken with j == throwAt. + expected = throwAt; + } + else + { + int sum = 0; + for (int i = 0; i < data.Length; i++) + { + sum += data[i]; + } + + expected = sum; + } + + Assert.Equal(expected, SumLiveAcrossCatch(data, throwAt)); + } +} diff --git a/src/tests/JIT/opt/Loops/PrimaryIVLiveInHandler.csproj b/src/tests/JIT/opt/Loops/PrimaryIVLiveInHandler.csproj new file mode 100644 index 00000000000000..b47c3e8e8d9f55 --- /dev/null +++ b/src/tests/JIT/opt/Loops/PrimaryIVLiveInHandler.csproj @@ -0,0 +1,9 @@ + + + PdbOnly + True + + + + +