[11] RFR(M): 8203915: Induction variable of over-unrolled loop conflicts with range checks

[Vladimir Kozlov]

Hi Tobias,

Can you look if PhaseIdealLoop::do_range_check() is executed for main loop in this case? If array 
length is known statically that code should produce check which should eliminate control flow for 
main loop and collapse it.

Your changes seems fine but I want to understand why existing mechanism does not work.

Thanks,
Vladimir

On 6/7/18 6:39 AM, Tobias Hartmann wrote:
> Hi,
> 
> please review the following patch:
> https://bugs.openjdk.java.net/browse/JDK-8203915
> http://cr.openjdk.java.net/~thartmann/8203915/webrev.00/
> 
> The general problem is that a range check predicated loop is over-unrolled such that the type of the
> loop induction variable is out of the statically known bounds of the array. As a result, the
> CastII/ConvI2L nodes are replaced by TOP and the memory graph is corrupted because the control path
> to this dead code is not removed. This is because there are no range checks in the loop but only
> predicates before the main loop.
> 
> For example, in TestOverunrolling::test5() we unroll the inner for-loop 16 times because the loop
> limit 'i' is not statically known. The last iArr access in the unrolled main loop body therefore has
> the type [6+stride-1, int] with stride = 16 which is out of the statically known array size 8. After
> vectorization, we crash in the matcher because the vectorized result += ... computation has a TOP
> memory input. Test3 and test4 trigger the same problem but fail at different stages during compilation.
> 
> This is very similar to what Roland fixed with skeleton predicates in JDK-8193130 [1]. However, this
> fix only added checks to verify that the initial value of the induction variable is in bounds but
> not that init+stride-1 is in bounds as well.
> 
> I've changed the skeleton predicate code to not only add a check for init <u length but also add
> another copy of the skeleton predicate to before the main loop. Whenever we now unroll the loop and
> therefore increase the stride, this predicate copy is updated to check init+stride-1 <u length. To
> prevent early folding of the expression, I left the Opaque1Nodes in (they are removed after loop
> optimizations).
> 
> The reason why this issue only showed up now (JDK 9 and 10 are also affected) is because
> vectorization of loops with safepoints was fixed by JDK-8200477 in JDK 11.
> 
> Thanks to Roland for discussing this!
> 
> Best regards,
> Tobias
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8193130
> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-December/027938.html
>