RFR (M): 7173340: C2: code cleanup: use PhaseIterGVN::replace_edge(Node*, int, Node*) where applicable
Vladimir Ivanov
vladimir.x.ivanov at oracle.com
Thu Jun 7 10:27:35 PDT 2012
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
More information about the hotspot-compiler-dev
mailing list