[9] RFR(S) 8065618: C2 RA incorrectly removes kill projections

[Rickard Bäckman]

Vladimir,

I think it would be great if the for loop that looks for the SCMemProj
could be moved out to be a function on it's own and just have 
if (n->is_MachProj() && n->has_SCMemProj()) {
  return false;
}

has_SCMemProj() could be replaced by something else.

Otherwise it looks good.

/R

On 11/21, Vladimir Kozlov wrote:
> http://cr.openjdk.java.net/~kvn/8065618/webrev/
> https://bugs.openjdk.java.net/browse/JDK-8065618
> 
> Some nodes have SCMemProj projection node to keep them alive even
> when their result is not used because they have memory side effects.
> For example, EncodeISOArrayNode and loadstore nodes like
> CompareAndSwapPNode. Generated code for such nodes may kill some
> registers and flags during execution. Killed registers are declared
> in 'effect' list in mach nodes definitions in .ad files. For each
> killed register MachProj node is created without use:
> 
>  112    encode_iso_array        ===  103  125  113  114  108  119
> 120  121  122  [[ 116  117  118  123  124  127  131  134  102  156
> ]] !jvms: ISO_8859_1$Encoder::encode @ bci:32 Test6896617::test @
> bci:26
>  116    MachProj        ===  112  [[]] #1
>  117    MachProj        ===  112  [[]] #2
>  118    MachProj        ===  112  [[]] #3
>  127    SCMemProj       ===  112  [[ 126  136  138  139  140  150
> 160 ]]   Memory: @BotPTR *+bot, idx=Bot; !jvms:
> ISO_8859_1$Encoder::encode @ bci:32 Test6896617::test @ bci:26
>  123    MachProj        ===  112  [[]] #4
>  124    MachProj        ===  112  [[]] #5
> 
> Register Allocator in PhaseChaitin::build_ifg_physical() removes
> nodes which are not used (dead). Nodes with
> 
> To keep such nodes alive when their result is not used there method
> PhaseChaitin::add_input_to_liveout() puts node's LiveRange on
> liveout list when the node has SCMemProj projection.
> 
> Unfortunately SCMemProj node could be placed in a block in such
> order that kill MachProj nodes are processed first (as in example
> above). When those MachProj are processed the def node looks like
> dead and these nodes are removed from graph. As result killed
> register is not marked as killed anymore and it could be used after
> code which kills/modifies it.
> 
> I was not able to write test with existing VM code. encodeISOArray()
> method can't be called directly - it is part of
> sun.nio.cs.ISO_8859_1$Encoder::encode() which calls it in a loop.
> There is enough code after encodeISOArray() call in encode() method
> which prevents using the same register after encode(). I have
> experimental project with intrinsics which hits this problem and I
> verified the fix with it.
> 
> Thanks,
> Vladimir
> 
> 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20141124/7a840889/signature.asc>