RFR (M): 7173340: C2: code cleanup: use PhaseIterGVN::replace_edge(Node*, int, Node*) where applicable
Vladimir Ivanov
vladimir.x.ivanov at oracle.com
Wed Jun 6 16:35:36 PDT 2012
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