Encapsulate live var analysis in its own class.#8856
Conversation
This change moves the implementation of live variable analysis from a single function into a class in which the per-block portion of the algorithm is contained in its own function. There is no functional change.
|
@CarolEidt @dotnet/jit-contrib PTAL |
|
Zero SPMI asm diffs due to this change? |
|
@sivarv: correct. |
src/jit/liveness.cpp
Outdated
| } | ||
|
|
||
| noway_assert(opts.compDbgCode && (info.compVarScopesCount > 0)); | ||
| /* For lvaKeepAliveAndReportThis methods, "m_compiler" has to be kept alive everywhere |
There was a problem hiding this comment.
Incorrect comment update - "m_compiler" should stay "this".
There was a problem hiding this comment.
That's what I get for '<,'>s/this/m_compiler/g... Nice catch.
src/jit/liveness.cpp
Outdated
| noway_assert(opts.compDbgCode && (info.compVarScopesCount > 0)); | ||
| /* For lvaKeepAliveAndReportThis methods, "m_compiler" has to be kept alive everywhere | ||
| Note that a function may end in a throw on an infinite loop (as opposed to a return). | ||
| "m_compiler" has to be alive everywhere even in such methods. */ |
|
It's great to see a bit of refactoring going in the JIT codebase 😄 |
CarolEidt
left a comment
There was a problem hiding this comment.
I suspect that the C++ compilers can optimize this to be as efficient as before, but given that the lclVar sets are now members of the class instance, it might be good to do a throughput measurement just to be certain.
LGTM modulo a couple of suggestions.
src/jit/liveness.cpp
Outdated
| } | ||
|
|
||
| for (block = fgLastBB; block; block = block->bbPrev) | ||
| void PerBlock(BasicBlock* block, bool updateInternalOnly, bool keepAliveThis) |
There was a problem hiding this comment.
I would prefer a more meaningful name for this. PerBlock() could be something that just returns the liveness info for the block, rather than computing it. How about PerBlockAnalysis?
There was a problem hiding this comment.
PerBlockAnalysis sounds good to me. Thanks for the suggestion!
| else | ||
| { | ||
| VarSetOps::AddElemD(this, liveOut, lvaTable[info.compThisArg].lvVarIndex); | ||
| VarSetOps::Assign(m_compiler, block->bbLiveIn, m_liveIn); |
| { | ||
| block->bbHeapLiveIn = m_heapLiveIn; | ||
| block->bbHeapLiveOut = m_heapLiveOut; | ||
| m_changed = true; |
There was a problem hiding this comment.
Does bbHeapLive change request PerBlockAnalysis iterations? Can we calculate bbHeapLive(In/Out) separately from live variables?
There was a problem hiding this comment.
It shouldn't change after all blocks have been visited, but updating it is essentially free.
|
The arm cross build failure appears to be infrastructural. @dotnet-bot test Windows_NT arm Cross Release Build |
Encapsulate live var analysis in its own class. Commit migrated from dotnet/coreclr@f4d2709
This change moves the implementation of live variable analysis from a
single function into a class in which the per-block portion of the
algorithm is contained in its own function. There is no functional
change.