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

[Vladimir Kozlov]

Good suggestion, Rickard.

I filed RFE:

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

Thanks,
Vladimir

On 11/24/14 12:28 AM, Rickard Bäckman wrote:
> 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
>>
>>
>>
>>