JDK-6591247 : C2 cleans up the merge point too early during SplitIf.
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 6u1,6u14
  • Priority: P4
  • Status: Closed
  • Resolution: Fixed
  • OS: generic,solaris,solaris_10
  • CPU: generic,x86,sparc
  • Submitted: 2007-08-09
  • Updated: 2012-06-26
  • Resolved: 2011-09-30
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7 JDK 8 Other
7u2Fixed 8Fixed hs22Fixed
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
EVALUATION See main CR
12-09-2011

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

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

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
09-08-2007