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

[Roland Westrelin]

> Reviewed.

Thanks for the review.

> 
> 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’ll do that before I push the change.

> 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             }

I’ll add the comments in the test case as well.

Roland.

> 
> 
> 
>> 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.
>