Request for reviews (L): 6674588: Escape Analysis: Improve Escape Analysis code

[John Rose]

On Mar 13, 2008, at 2:42 PM, Vladimir Kozlov wrote:

> I updated changes and webrev.

Here's the rest of my review.  I hope it helps!

-- John


--- escape.cpp

+        // See comments above.
+        Node* unique_use = use->raw_out(0);

That looks like a factorization opportunity:
    // big comment ...
    Node* find_second_addp(Node* use) { if (...)  return use- 
 >unique_out(); else return NULL; }






+    if(!_processed.test(n->_idx) && n->is_AddP())
+      cg_worklist.append(n->_idx);

I looked but did not find how _processed could be set for the AddP node.
What sorts of a AddPs are already processed at this early point?

+void ConnectionGraph::record_for_escape_analysis(Node *n,  
PhaseTransform *phase) {
+  switch (n->Opcode()) {

It seems like almost every case in that big switch includes the  
following:
    ptadr->_node = n;
    ptadr->set_node_type(PointsToNode::(something));
    set_escape_state(n->_idx, PointsToNode::(something else));
    _processed.set(n->_idx);

Can those steps be factored out of the switch somehow,
so it's easy to tell where the irregularities are (if there are any)?
(It looks like you found on on case Op_LoadKlass.)
The pattern could be:

    int ntype = (something default);
    int estate = (something else default);
    bool defer = false;
    _processed.set(n->_idx);
    switch (n->Opcode()) {
       ...
      default:  return;  // nothing to do
    }
    ptadr->_node = n;
    ptadr->set_node_type(ntype);
    set_escape_state(n->_idx, estate);
    if (defer) {
      _deferred.push(n);
      _processed.clear(n->_idx);
    }
    return;

By the way, the name _deferred confuses me because it makes me think  
of a kind of CG edge.
Should be "_delayed_worklist" or "_pass_2_worklist" or some such.


+        process_call_result(n->as_Proj(), phase);
+        if (!_processed.test(n->_idx)) {
+          _deferred.push(n);

Can _processed.test(n) ever be true here?
(I.e., isn't this an unconditional push to _deferred?)

+        add_deferred_edge_to_fields(n->_idx, pt, address_offset(adr,  
phase));

FWIW, address_offset(adr, phase) is loop invariant here.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20080313/abff6967/attachment.html