RFR(S): 8087128 C2: Disallow definition split on MachCopySpill nodes
vladimir.kozlov at oracle.com
Tue Jan 29 22:48:43 UTC 2019
Thank you, Nils
Looks good to me.
Please run hs-precheckin-comp testing too.
On 1/28/19 7:15 AM, Nils Eliasson wrote:
> We have a problem that we sometimes hit an assert in reg_split.cpp.
> We have a block that looks like this:
> 1262: # B1264 B1263 <- N7283 Freq: 0,0927179
> 7282 Region === 7282 1704 [[ 7282 1702 1715 ]]
> 11500 MemToRegSpillCopy === _ 11190 [[ 9184 ]] Oop:com/sun/tools/javac/$
> 9184 DefinitionSpillCopy === _ 11500 [[ 9185 1702 11503 11505 ]] Oop:$
> 11501 MemToRegSpillCopy === _ 9169 [[ 11665 11506 11504 11502 ]] Oop$
> 11645 MemToRegSpillCopy === _ 9167 [[ 9182 ]] Oop:com/sun/tools/javac/c$
> 9182 DefinitionSpillCopy === _ 11645 [[ 1702 11646 11647 11648 ]] Oop$
> 1715 checkCastPP === 7282 1716 [[ 1702 ]] java/util/HashMap$TreeNode:NotN$
> 9185 BoundSpillCopy === _ 9184 [[ 1702 ]] Oop:com/sun/tools/javac/code/Symb$
> 11665 RegToMemSpillCopy === _ 11501 [[ 1702 ]] Oop:com/sun/tools/javac/$
> 1702 CallStaticJavaDirect === 7282 185 182 16 0 1715 187 9185 11665 $
> 1703 MachProj === 1702 [] #10006/fat
> 11501 "MemToRegSpillCopy" has one use in this block, "11665 RegToMemSpillCopy", and three uses in other blocks. The use
> "11665 RegToMemSpillCopy" is used by "1702 CallStaticJavaDirect".
> We hit the assert when processing 11501 in PhaseChaitin::Split.
> We are in the "Handle DEFS" section of the split routine. We only get here if the live range has been marked as spilled
> when the coloring have ran out of colors. There are two code paths, one default, where we just record the def and
> updates the side tables, and one where we do a definition split on the live range. This split is guarded by several
> conditions. In this case we get here by having a register mask that is only regs (UP) and by being in a high pressure
> region. Everything seems ok, so why don't we end up with MachSpillCopies here more often?
> Also - one of the 4 uses is in this block (11655) and it's a reg-to-mem already. It doesn't make much sense to add even
> more spills here. Why does this happen?
> UseFPUForSpilling added a restriction to coalesing - it skips coalescing when the two live ranges have different
> pressure. The reason for this is that with FPU spilling, the possible extra spilling is for "free". (I can't find any
> documentation on benchmarks where this is beneficial though.) The downside is that we get longer spill chains like:
> DefSpill-memToReg-RegToMem-MemToReg, that doesn't collapse, because of pressure changes. This may cause the live ranges
> defined by memToReg-nodes to become spilled, and if we are in a high-pressure region - we hit the assert.
> So my conclusion is that nothing is really wrong. Everything still works without the assert. The spill-chains are
> unnecessary long, but only because we have chosen to restrict the coalescing. But we shouldn't split the spill-nodes
> even more. In the next iteration the coalescing within the block will have reduced the chains, and later a proper
> coloring will be found.
> My solution is that we prevent the MachSpillCopies (only Mem-To-Regs can end up here) from being split again. This is ok
> - because this is exactly what would have happened if we would have been in a low pressure region.
> I have done some measurements and it doesn't increase the number of spill-iterations.
More information about the hotspot-compiler-dev