Uh oh!
There was an error while loading. Please reload this page.
JIT: Invalidate loop table after unrolling and dominators after MarkLocalVars - #95854
Conversation
Null out the loop table as part of setting optLoopTableValid = false. This reveals a few cases where we were using it even after invalidation. A few diffs expected from the fgCanCompactBlocks change because we now compact some loop headers.
ghost
commented
Dec 11, 2023
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsNull out the loop table as part of setting optLoopTableValid = false. This reveals a few cases where we were using it even after invalidation. A few diffs expected from the
|
| if (m_comp->optLoopTableValid) | ||
| { | ||
| m_comp->fgUpdateLoopsAfterCompacting(m_b1, m_b3); | ||
| m_comp->fgUpdateLoopsAfterCompacting(m_b1, m_b2); | ||
| if (optReturnBlock) | ||
| { | ||
| m_comp->fgUpdateLoopsAfterCompacting(m_b1, m_b3); | ||
| } |
There was a problem hiding this comment.
Some of these checks are always false, like in this case... I'll remove them all as part of the big code deletion in the future.
| DoPhase(this, PHASE_MARK_LOCAL_VARS, &Compiler::lvaMarkLocalVars); | ||
| // Dominator and reachability sets are no longer valid. | ||
| fgDomsComputed = false; | ||
| fgCompactRenumberQuirk = true; | ||
There was a problem hiding this comment.
lvaMarkLocalVars uses Compiler::IsDominatedByExceptionalEntry to set LclVarDsc::lvVolatileHint. That is used by VN-based copy prop as part of the heuristic. My next step will be to compute SSA's dominators after unrolling above and then use those dominators to compute this heuristic instead. That will allow us to move fgDomsComputed up to where the loop table is invalidated as well.
| else | ||
| { | ||
| // TODO-Quirk: Remove | ||
| JITDUMP("Renumbering " FMT_BB " to be " FMT_BB " for a quirk\n", block->bbNum, bNext->bbNum); | ||
| } |
There was a problem hiding this comment.
This quirk has surprisingly large diffs.
jakobbotsch
commented
Dec 11, 2023
@BruceForstall feel free to take a look at this one if you have time once the diffs are done. I need some extra work for |
jakobbotsch
commented
Dec 12, 2023
cc @dotnet/jit-contrib PTAL @BruceForstall Surprisingly large TP wins with the win-x64 host compiler, but seems to be some MSVC related oddity since the other targets don't see the same. The detailed diff shows that it is from the Base:151246609045, Diff:150947499733, -0.1978%79307193:+6.15%:16.62%:+0.0524%:public: bool __cdecl Compiler::fgUpdateFlowGraph(bool, bool) 6316694:+173.37%:1.32%:+0.0042%:public: void __cdecl Compiler::optFindNewLoops(void) 1122411:+0.49%:0.24%:+0.0007%:public: void __cdecl Compiler::compInit(classArenaAllocator*, struct CORINFO_METHOD_STRUCT_*, classICorJitInfo*, struct CORINFO_METHOD_INFO*, struct InlineInfo*)
635604:+0.39%:0.13%:+0.0004%:public: void __cdecl Compiler::fgCompactBlocks(struct BasicBlock*, struct BasicBlock*) 614159:+0.07%:0.13%:+0.0004%:public: void __cdecl Compiler::fgPerBlockLocalVarLiveness(void) -751605:-100.00%:0.16%:-0.0005%:public: enumPhaseStatus __cdecl Compiler::optClearLoopIterInfo(void) -1193033:-2.48%:0.25%:-0.0008%:public: classBasicBlock::BBSuccList __cdecl BasicBlock::Succs(void) const -2454913:-0.68%:0.51%:-0.0016%: void __cdecl DoPhase(classCompiler*, enumPhases, enumPhaseStatus (__cdecl Compiler::*)(void)) -2580058:-2.82%:0.54%:-0.0017%:public: bool __cdecl Compiler::fgExpandRarelyRunBlocks(void) -4782745:-56.62%:1.00%:-0.0032%:protected: void __cdecl Compiler::optUpdateLoopsBeforeRemoveBlock(struct BasicBlock*, bool) -5065556:-0.33%:1.06%:-0.0033%: memset -10648527:-22.10%:2.23%:-0.0070%:public: bool __cdecl Compiler::optJumpThreadPhi(struct BasicBlock*const, struct GenTree*, unsigned int) -14403526:-9.11%:3.02%:-0.0095%:public: enumCompiler::FoldResult __cdecl Compiler::fgFoldConditional(struct BasicBlock*) -18271428:-100.00%:3.83%:-0.0121%:public: void __cdecl Compiler::fgUpdateLoopsAfterCompacting(struct BasicBlock*, struct BasicBlock*) -326814374:-89.09%:68.47%:-0.2161%:public: bool __cdecl Compiler::fgCanCompactBlocks(struct BasicBlock*, struct BasicBlock*) |
Invalidate the loop table right after unrolling. Invalidate dominators right after MarkLocalVars.
Diffs come from two sources:
optUnmarkLoopBlocksthat would unscale the loop blocksAs always there are also quirks that I'll remove separately.