RFR (M): 7173340: C2: code cleanup: use PhaseIterGVN::replace_edge(Node*, int, Node*) where applicable

[Vladimir Kozlov]

Thank you.

Vladimir

Vladimir Ivanov wrote:
> Vladimir,
> 
> FYI, CTW testing is finished. Results are clean.
> 
> Best regards,
> Vladimir Ivanov
> 
> On 06/07/12 03:35, Vladimir Ivanov wrote:
>> Vladimir,
>>
>> Thanks for the review and for catching the wrong code! It was caused by
>> a last minute change to eliminate multiple rehash requests from
>> consequent replace_input_of calls. I was sure I couldn't break anything.
>> How wrong I was... =)
>>
>> Updated webrev with all your suggestions incorporated:
>> http://cr.openjdk.java.net/~vi/7173340/webrev.02/
>>
>> Testing of the latest changes are in progress. Will notify you when it's
>> finished.
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> On 6/6/12 10:15 PM, Vladimir Kozlov wrote:
>>> Vladimir,
>>>
>>> Next code is wrong, use_clone edge update should be after use node
>>> update since it is used there:
>>>
>>> ! _igvn.replace_edge_of(use_clone, LoopNode::LoopBackControl, C->top());
>>> ! _igvn.rehash_node_delayed(use); // Multiple edge updates
>>> use->set_req(LoopNode::EntryControl,
>>> use_clone->in(LoopNode::LoopBackControl));
>>> use->set_req(LoopNode::LoopBackControl, C->top());
>>>
>>> Could you also define rehash_node_delayed() first and use it in
>>> replace_edge_of() and delete_edge_of() methods since it is not required
>>> to have set_req() and del_req() before _worklist.push()?
>>>
>>> It was my suggestion to use 'edge' in new methods name but I think it is
>>> not accurate. Methods update only input edges so it would be better to
>>> call methods replace_input_of() and delete_input_of().
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> Vladimir Ivanov wrote:
>>>> http://cr.openjdk.java.net/~vi/7173340/webrev.01/
>>>>
>>>> The idea is to replace the following patterns with a single call to
>>>> a corresponding method from PhaseIterGVN:
>>>> 1) igvn.hash_delete(n);
>>>> n->set_req(i, in);
>>>> igvn._worklist.push(n);
>>>>
>>>> 2) hash_delete(n);
>>>> n->del_req(i);
>>>> _worklist.push(n);
>>>>
>>>> 3) hash_delete(n);
>>>> _worklist.push(n);
>>>> to
>>>> 1) PhaseIterGVN::replace_edge_of(Node*, int, Node*)
>>>> 2) PhaseIterGVN::delete_edge_of(Node*, int)
>>>> 3) PhaseIterGVN::rehash_node_delayed(Node*)
>>>>
>>>> I reordered code in some places, but it should be safe. Please, confirm.
>>>>
>>>> Testing: VM tests, VM regression tests
>>>>
>>>> Thanks!
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov