RFR(XS): 8048170: Test closed/java/text/Normalizer/ConformanceTest.java failed

John Rose john.r.rose at oracle.com
Tue Dec 2 23:54:36 UTC 2014


I wish we had a less ambiguous way to mark the overloaded 'If' nodes.

I suggest adding a comment linking your change to IfNode and the concept of smearing:

   + // If this is a range check (IfNode::is_range_check), do not reorder because
   + // Compile::allow_range_check_smearing might have changed the check.
      return; // Let IGVN transformation change control dependence.

I suggest adding comments to the test case so that it can be read without referring to the bug report.

Perhaps (although I'm not certain my comments are correct!):

  38                 if (array[i] < 0) {      // range check (i+0) dominates equivalent check below
  49.9          // IfNode::Ideal will rewrite some range checks if Compile::allow_range_check_smearing
  50             if (array[i-1] == 9) {     // range check (i-1) changed to off_lo = (i-3)
  51                 int res = array[i-3];  // range check (i-3) changed to off_hi = (i+0)
  51.1              // the previous access might be hoisted by PhaseIdealLoop::split_if_with_blocks_post
  51.2              // because it appears to have the same guard, but it also depends on the previous guard
  52                 res += array[i];        // removed redundant range check
  53                 res += array[i-2];     // removed redundant range check
  54                 return res;
  55             }

> On Dec 1, 2014, at 7:11 AM, Roland Westrelin <roland.westrelin at oracle.com> wrote:
> http://cr.openjdk.java.net/~roland/8048170/webrev.00/
> After range check smearing, an array access may depend on multiple range checks to be valid. PhaseIdealLoop::split_if_with_blocks_post() shouldn’t replace an IF by a dominated IF for the same test for range checks otherwise it can make an array access bypass a range check that it depends on.
> Roland.

More information about the hotspot-compiler-dev mailing list