RFR(S): C2: Disallow definition split on MachCopySpill nodes

[Nils Eliasson]

Hi,

We have a problem that we sometimes hit an assert in reg_split.cpp.

https://bugs.openjdk.java.net/browse/JDK-8087128

http://cr.openjdk.java.net/~neliasso/8087128/webrev.01/

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.

Regards,

Nils