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

[Tobias Hartmann]

Hi,

final testing revealed a problem with my patch: if the stride is negative, we need check "2 * stride
+ 1" instead of "2 * stride - 1". Here's the incremental webrev:
http://cr.openjdk.java.net/~thartmann/8203915/webrev.inc/

All tests pass and benchmarking shows no regressions.

Thanks,
Tobias

On 07.06.2018 15:39, 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
>