JDK-6362268 : implicit-null-check instance causes bad spilling
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 1.4.2_12,5.0u6
  • Priority: P4
  • Status: Closed
  • Resolution: Duplicate
  • OS: generic
  • CPU: generic
  • Submitted: 2005-12-13
  • Updated: 2010-04-02
  • Resolved: 2005-12-13
Related Reports
Duplicate :  
Description
This bug is found in the implicit-null-check creation code.  Here's a sample piece of code which can trigger the bug (assuming the allocator does enough bad spilling much much later):

  if( ptr != null ) { // Null Check to be optimized, when load below is hoisted
    S1;
  } else {
    S2;
  }
  S3; // Arbitrary huge control flow & code, to cause spilling
  if( ptr != null ) {
    ... = ptr._field; // Load op to be hoisted
  }

After lcm.cpp hoists to cover, AND the allocator gets done spilling we have:
  if( (tmp=ptr._field) != null ) { // Null Check optimized
    [stack_slot+8] = tmp; // spill down
    S1;
  } else {
    S2;
  }
  S3; // Arbitrary huge control flow & code, to cause spilling
  if( ptr != null ) {
    tmp = [stack_slot+8]; // spill up
    ... = tmp; // Load op to be hoisted
  }

And now the allocator notices that in the S2 arm of the code, there's no DEF of [stack_slot+8] and barfs.

Here's the new code, search for "Stores can be hoisted..." in lcm.cpp:implicit_null_check:

    // Check ctrl input to see if the null-check dominates the memory op
    Block *cb = bbs[mach->_idx];
    Node *last_cb_ctrl = cb->_nodes[0];
    cb = cb->_idom;             // Always hoist at least 1 block
    // Stores can be hoisted only one block
    if( !was_store ) {
      while( cb->_dom_depth > _dom_depth ) {
        last_cb_ctrl = cb->_nodes[0];
        cb = cb->_idom;         // Hoist loads as far as we want
      }
    }
    if( cb != this ) continue;
    // This whole routine attempts to hoist a LoadNode up above a null-check,
    // to allow the LoadNode to do the null-check "for free" in the hardware.
    // However, conceptually the LoadNode only returns a value if the test
    // passes; if the base address is NULL the LoadNode never completes
    // (faults instead).  In other words, the LoadNode's value is only DEF'd
    // on one arm of the conditional.  If the loaded value is then used below
    // the merge point of both paths, we potentially have a USE with no DEF.
    // The register allocator can end up spilling the LoadNode's result, past
    // the NULL-check, and thus down one arm only of the conditional.  Later
    // the allocator asserts because there is no DEF on the other arm.  Fixed
    // by not hoisting if we would hoist above the post-dominating point.
    if( last_cb_ctrl->is_Region() && last_cb_ctrl->req() > 2 ) continue;

Comments
EVALUATION This has already been direcly addressed in 6332641. SubCRs have been added there to backport the fix to 5.0 and 1.4.2 trains.
13-12-2005