[vector] RFR 8221816: IndexOutOfBoundsException for fromArray/intoArray with unset mask lanes - was: RE: IndexOutOfBoundsException with unset mask lanes

[Vladimir Ivanov]

Hi Joshua,

What kind of performance testing have you done on it?

Having a conditional branch with forEach() call on slow path will cause 
problems to C2 if it can't prove access is always in-bounds and prune 
the slow path.

Overall, I don't see an acceptable way to fix it purely on JDK side.
I like how you optimize for common case with a stricter bounds check, 
but more work is needed to reliably turn slow path into an uncommon trap.

+        if (ax + LENGTH <= a.length) {

VectorIntrinsics.checkIndex() has a more optimal implementation: it uses 
Objects.checkIndex() which internally relies on VM intrinsic.

It looks promising to introduce a variant of 
VectorIntrinsics.checkIndex() which is used to guard a fast path and is 
annotated with a JIT-compiler hint (akin to 
java.lang.invoke.MethodHandleImpl.profileBoolean() [1], but without 
profiling logic) to override bytecode profiling info, so JIT always puts 
an uncommon trap on the false branch.

Best regards,
Vladimir Ivanov

[1] 
http://hg.openjdk.java.net/panama/dev/file/ccfafe96478e/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java#l695

On 03/04/2019 03:03, Joshua Zhu (Arm Technology China) wrote:
> Hi,
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8221816
> Webrev: http://cr.openjdk.java.net/~jzhu/vectorapi/8221816.OOB/webrev.00/
> 
> (This is a rebase of original webrev [1].)
> 
> I have a patch to fix IndexOutOfBoundsException for
> fromArray/intoArray/gather/scatter API with unset mask lanes.
> To align with API doc, IndexOutOfBoundsException should only be thrown for
> any illegal vector lane index where the mask at lane is set.
> 
> Also I write a template to generate test cases to cover this kind of exception testing.
> 
> This patch has been verified by testing.
> 
> [1] http://mail.openjdk.java.net/pipermail/panama-dev/2019-March/004812.html
> 
> Best Regards,
> Joshua Zhu
>