Problem is observed on 1.5.0.08.FCS

Encountered a compiler crash, while compiling following method:
com.sun.jndi.ldap.sasl.LdapSasl::saslBind(Lcom/sun/jndi/ldap/LdapClient;
Lcom/sun/jndi/ldap/Connection;Ljava/lang/String;Ljava/lang/String;
Ljava/lang/Object;Ljava/lang/String;Ljava/util/Hashtable;
[Ljavax/naming/ldap/Control;)Lcom/sun/jndi/ldap/LdapResult; (437 bytes)

Source Code Looks like:
------------------------
     74 public static LdapResult saslBind(LdapClient clnt,
Connection conn,
     75 String server, String dn, Object pw,
     76 String authMech, Hashtable env, Control[] bindCtls)
     77 throws IOException, NamingException {
     78
     79 SaslClient saslClnt = null;
     80 boolean cleanupHandler = false;
     81
     82 // Use supplied callback handler or create default
     83 CallbackHandler cbh =
     84 (env != null) ?
(CallbackHandler)env.get(SASL_CALLBACK) : null;
     85 if (cbh == null) {
     86 cbh = new DefaultCallbackHandler(dn, pw,
(String)env.get(SASL_REALM));
     87 cleanupHandler = true;
     88 }
     89
     90 // Prepare parameters for creating SASL client
     91 String authzId = (env != null) ?
(String)env.get(SASL_AUTHZ_ID) : null;
     92 String[] mechs = getSaslMechanismNames(authMech);
     93 . . . . .
     94 Continues on . . . .
     95 . . . . .

Inlining Information:
---------------------
      @ 49 com.sun.jndi.ldap.sasl.DefaultCallbackHandler::<init>
inline (hot)
        @ 1 java.lang.Object::<init> inline (hot)
        @ 26 java.lang.String::toCharArray inline (hot)
          @ 15 java.lang.String::getChars0 inline (hot)
  Inlining intrinsic static void java.lang.System.arraycopy(jobject,
jint, jobject, jint, jint)
        @ 82 java.lang.String::<init> inline (hot)
          @ 6 java.lang.String::<init> inline (hot)
            @ 1 java.lang.Object::<init> inline (hot)
            @ 22 java.lang.String::checkBounds inline (hot)
  Inlining intrinsic virtual jint java.lang.String.compareTo(jobject)
  Inlining intrinsic virtual jint java.lang.String.compareTo(jobject)
            @ 89 java.lang.StringCoding::decode hot method too big
        @ 90 java.lang.String::toCharArray inline (hot)
          @ 15 java.lang.String::getChars0 inline (hot)
  Inlining intrinsic static void java.lang.System.arraycopy(jobject,
jint, jobject, jint, jint)
      @ 80 com.sun.jndi.ldap.sasl.LdapSasl::getSaslMechanismNames
inline (hot)
. . . . . .
Continues on . . . .
. . . . . . .


Note: We were able to see this level of inlining, because of our internal
enhancement enabled through option XX:+OptimisticInlining. It purpose
is to
Force inlining call-sites regardless its call profile. If you are
interested,
we can share the source changes. But I have provided the calling sequence,
for reference also.

Problem is observed during the optimizations of if-block, from line
85 to 88 in the Java source. I have attached a hand drawn picture of
the IR after parsing.

A Very Low-Level And Detailed Background:
-----------------------------------------
In the graph RegionNode (#124) is like you are at line no 85 in
Java Source. I noticed, that the CheckCastPPNode (# 117) is getting
input from CastPP, i.e. we are feeding a non-null value. Hence
effectively, we may say that, if the value of env is not null,
then we are feeding a non-null value for cbh. As incase,
return value of CallDynamicJava( # 48) was null we will hit
an uncommon-trap. This looked like was part of an optimization
that can be disabled using option XX-UncommonNullCast. By
default UncommonNullCast is enabled. The sub-graph between
IfNode (# 72) and RegionNode (# 117) was generated by
GraphKit::gen_checkcast().

Now, after looking at the usage of CheckCastPP and RegionNode
(#124), We can say that, control will never reach IfNode (#132)
if the return value of CallDynamicJava(#48) is null
(due to uncommon trap). In other words, IfNode (# 132) was
only reachable from IfTrue(#45), which implies that value of
env has to be null.

This became more clear after first pass of PhaseIdealLoop.
I noticed that the IfNode (# 132), that is control dependent
on Region (124) was split-up. In Fig-2, I have this sub-graph
created, that reflects the changes made after if-splitting.
After attempting to split-up IfNode( #132), the new If nodes
produced, no longer are getting inputs from BoolNode, rather
are being seeded with a constant-one and constant-zero,
respectively. In this figure, we see that the second merge
point of RegionNode( #20834) is a dead path, and that is
because of the following input order.
IfNode( Bool( Cmp ( Phi ( NULL, (CheckCastPP (CastPP)) ), NULL) ) )
Because of CastPP, the type of CheckCastPP will be NOTNULL

All that said, it becomes more evident that the receiver of
CallDynamicJava (#194) will be null and we will always go through
the exception path and hence fall-through path will be dead.

Gist Of the problem:
---------------------
During PhaseIdealLoop, and particularly during if-splitting, we
added couple of new nodes in the IGVN-worklist, which includes
RegionNode (# 20834), PhiNode(#20840), etc. At end of
PhaseIdealLoop, when we process the IGVN worklist, we recognized,
that _in[2] of Region(#20834) was dead, and hence we also change
the _in[2] of the PhiNode(# 20840) to TOP. This Phi will again
be transformed to a ConPNode, which is equivalent to NULL. This
changed the inputs of CallDynamicJava(#194) and it got placed
onto the worklist, and was left without any further transformation.
At this point, we have identified a potential dead path, but
is left un-noticed. Ideally, To me it seems more reasonable,
to do the best effort, to cleanup all the graph, during the
same IGVN pass, based on all the information we have gathered
so far. CatchNode::Value function does the trick of recognizing
such CallNodes and if the receiver is null, it appropriately
sets the type of its projection, but we never get to process
the associated CachNode during IGVN, and the whole sub-graph
under the fall-through projection remained in the IR without
being detected as dead, during the currently executed pass
of IGVN.


What happened next was:
------------------------
Next point when this dead path is recognized will be during
PhaseCCP. In this phase as we seem to do a global top-down
traversal and evaluate the types of each reachable node. Any
node that was not reachable during PhaseCCP::analyze() is
considered dead. Later on during PhaseCCP transformation
(traversing bottom-up) we start to remove/subsume any dead
node as we encounter. In order to remove dead nodes we seem
to make use of function remove_globally_dead_node(). I
think we could still have managed to remove this dead sub-graph.
In this function we check if after disconnecting input edge
of a "dead" node, there is no use of "in" node, we consider
"in" to be also dead. But we seem to take no action, if
in->outcount() != 0 and the type is TOP. In my case, "dead"
node was a CFG node and while looking at the nodes merging at
this dead-CFG-node, one of the input edges were comming from a
If-projection i.e. IfFalseNode, (which is also a loop exit point).
If the other If-projection i.e. IfTrueNode is a backedge, we will
have a situation where we will end up removing the exit path of
the loop, but the LoopEndNode which is also known to be dead, will
encounter no action. And hence are left with a subgraph, rather
a loop region, with no exit path.


During my investigation, I noticed this un-deleted loop
first. I made small change, to fix to ensure that the
un-reachible graph is properly deleted. I made a small change,
but later on considered that as a workaround.

What I did was:
Instead of leaving this case un-noticed, I push this "in"
node on to the IGVN worklist. (Ref: See my
workaround ahead)
Here was the change I made to see the impact.

void PhaseIterGVN::remove_globally_dead_node( Node *dead ) {
  assert(dead != C->root(), "killing root, eh?");
  if (dead->is_top()) return;
  NOT_PRODUCT( set_progress(); )
  // Remove from iterative worklist
  _worklist.remove(dead);
  // Remove from hash table
  _table.hash_delete( dead );
  // Smash all inputs to 'dead', isolating him completely
  for( uint i = 0; i < dead->req(); i++ ) {
    Node *in = dead->in(i);
    if( in ) { // Points to something?
      dead->set_req(i,NULL); // Kill the edge
      if (in->outcnt() == 0 && in != C->top()) {// Made input go dead?
        remove_dead_node(in); // Recursively remove
#ifdef HP
      // Ref: JAGag01006 / D98
      // Without this, a potential dead path merging into the current dead
      // node may not be identified.
      } else if (in->outcnt() != 0 && type_or_null(in) == Type::TOP) {
        _worklist.push(in);
#endif
      } else if (in->outcnt() <= 2 && dead->is_Phi()) {
        if( in->Opcode() == Op_Region )
          _worklist.push(in);
        else if( in->is_Store() ) {
          DUIterator_Fast imax, i = in->fast_outs(imax);
          _worklist.push(in->fast_out(i));
          i++;
          if(in->outcnt() == 2) {
            _worklist.push(in->fast_out(i));
            i++;
          }
          assert(!(i < imax), "sanity");
        }
      }
    }
  }

  // Aggressively kill globally dead uses
  // (Cannot use DUIterator_Last because of the indefinite number
  // of edge deletions per loop trip.)
  while (dead->outcnt() > 0) {
    remove_globally_dead_node(dead->raw_out(0));
  }
}

Remarks
-------
The change I made, will not remove the node rightaway. Instead I
consider it a dead-candidate, and push it for IGVN processeing.
since the analysis is monotonic, once a fixed-point is reached,
any further attempt to transform a node should be a NOP.
That said, I also think, that this still looked like a workaround
to some uncaught problem. Infact that was indeed the case, as I
had already left a dead fall-through-path undetected during an
earlier IGVN pass.