United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-6591247 C2 cleans up the merge point too early during SplitIf.
JDK-6591247 : C2 cleans up the merge point too early during SplitIf.

Details
Type:
Bug
Submit Date:
2007-08-09
Status:
Closed
Updated Date:
2012-06-26
Project Name:
JDK
Resolved Date:
2011-09-30
Component:
hotspot
OS:
solaris,generic,solaris_10
Sub-Component:
compiler
CPU:
x86,sparc,generic
Priority:
P4
Resolution:
Fixed
Affected Versions:
6u1,6u14
Fixed Versions:
hs22 (b05)

Related Reports
Backport:
Backport:

Sub Tasks

Description
Do not clean up the merge point too early during C2 SplitIf.  The cleaned merge point looks dead and confuses the lazy skip-through-dead-control-flow data structure.  Clean it up after the last possible use of it.
From hs_err log sent by Bond Desk:

V  [libjvm.so+0x322a46]  Node*PhaseIdealLoop::get_ctrl_no_update(Node*)const+0x26
V  [libjvm.so+0x9e7836] bool PhaseIdealLoop::split_up(Node*,Node*,Node*)+0x856
V  [libjvm.so+0x405393] void PhaseIdealLoop::do_split_if(Node*)+0x153
V  [libjvm.so+0x3285cd] void PhaseIdealLoop::split_if_with_blocks_post(Node*)+0x52d
V  [libjvm.so+0x41008a] void PhaseIdealLoop::split_if_with_blocks(VectorSet&,Node_Stack&)+0x4ca
V  [libjvm.so+0x902e3b] PhaseIdealLoop::PhaseIdealLoop(PhaseIterGVN&,const PhaseIdealLoop*,bool)+0xa4b
V  [libjvm.so+0x439826] void Compile::Optimize()+0x6b6
V  [libjvm.so+0x63a49d] Compile::Compile(ciEnv*,C2Compiler*,ciMethod*,int,bool,bool)+0xc9d
V  [libjvm.so+0x436d07] void C2Compiler::compile_method(ciEnv*,ciMethod*,int)+0x87
V  [libjvm.so+0x436866] void CompileBroker::invoke_compiler_on_method(CompileTask*)+0x746
V  [libjvm.so+0x4bfcf8] void CompileBroker::compiler_thread_loop()+0x6b8
V  [libjvm.so+0x4bc129] void compiler_thread_entry(JavaThread*,Thread*)+0x9
V  [libjvm.so+0x47b01d] void JavaThread::thread_main_inner()+0x4d
V  [libjvm.so+0x47adca] void JavaThread::run()+0x10a
V  [libjvm.so+0x96d059] java_start+0x1f9
C  [libc.so.1+0xd504b]  _thr_slot_offset+0x31b
C  [libc.so.1+0xd5280]  _thr_slot_offset+0x550

                                    

Comments
SUGGESTED FIX

==== //java/main-dev/java/hotspot5/src/share/vm/opto/split_if.cpp#7 - /home/cliffc/HotSpot/cliffc-bugs/hotspot5/src/share/vm/opto/split_if.cpp ====
@@ -534,19 +534,14 @@
   region_cache.lru_insert( new_false, new_false );
   region_cache.lru_insert( new_true , new_true  );
   // Now handle all uses of the splitting block
-  for (DUIterator_Last kmin, k = region->last_outs(kmin); k >= kmin; --k) {
-    Node* phi = region->last_out(k);
+  for (DUIterator_Fast kmax, k = region->fast_outs(kmax); k < kmax; k++) {
+    Node* phi = region->fast_out(k);
     if( !phi->in(0) ) {         // Dead phi?  Remove it
       _igvn.remove_dead_node(phi);
-      continue;
-    }
-    assert( phi->in(0) == region, "" );
-    if( phi == region ) {       // Found the self-reference
-      phi->set_req(0, NULL);
-      continue;                 // Break the self-cycle
-    }
-    // Expected common case: Phi hanging off of Region
-    if( phi->is_Phi() ) {
+    } else if( phi == region ) { // Found the self-reference
+      continue;                 // No roll-back of DUIterator
+    } else if( phi->is_Phi() ) { // Expected common case: Phi hanging off of Region
+      assert0( phi->in(0) == region );
       assert0( phi->in(LoopNode::LoopBackControl) != phi );
       // Need a per-def cache.  Phi represents a def, so make a cache
       small_cache phi_cache;
@@ -561,20 +556,25 @@
         // 2-element cache to handle multiple uses from the same block.
         handle_use( use, phi, &phi_cache, iff_replacement, new_false, new_true, old_false, old_true );
       } // End of while phi has uses
-      
-      // Because handle_use might relocate region->_out,
-      // we must refresh the iterator.
-      k = region->last_outs(kmin);
 
       // Remove the dead Phi
       _igvn.remove_dead_node( phi );
 
     } else {
+      assert0( phi->in(0) == region );
       // Random memory op guarded by Region.  Compute new DEF for USE.
       handle_use( phi, region, &region_cache, iff_replacement, new_false, new_true, old_false, old_true );
     }
+    // Every path above deletes a use of the region, except for the region
+    // self-cycle (which is needed by handle_use calling get_early_ctrl
+    // calling get_ctrl_no_assert calling get_ctrl_no_update looking for dead
+    // regions).  So roll back the DUIterator innards.
+    --k;
+    --kmax;
+  } // End of while merge point has phis
 
-  } // End of while merge point has phis
+  assert0( region->outcnt() == 1 ); // Just Self on the Region
+  region->set_req(0, NULL);     // Break the self-cycle
 
   // Any leftover bits in the splitting block must not have depended on local
   // Phi inputs (these have already been split-up).  Hence it's safe to hoist
                                     
2007-08-09
EVALUATION

http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/8805f8c1e23e
                                     
2011-08-27
EVALUATION

http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/rev/8805f8c1e23e
                                     
2011-09-08
EVALUATION

See main CR
                                     
2011-09-12



Hardware and Software, Engineered to Work Together