[jdk17] RFR: 8268293: VectorAPI cast operation on mask and shuffle is broken [v6]

Paul Sandoz psandoz at openjdk.java.net
Fri Jun 18 17:58:43 UTC 2021


On Fri, 18 Jun 2021 07:38:54 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Existing implementation of VectoMask.cast and VectorShuffle.cast operation do not generate correct mask compliant with the species passed as the argument.
>> 
>> This patch fixes the implementation and adds the corresponding cast operation test cases to VectorAPI regression suite.
>
> Jatin Bhateja has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
> 
>  - 8268293: Review comments resolution.
>  - Merge http://github.com/openjdk/jdk17 into JDK-8268293
>  - 8268293: Review comments resolution.
>  - 8268293: Removing unneeded changes from AbstractVectorTest.java
>  - 8268293: Reinstating accidentally removed tests.
>  - 8268293: Moving new tests to VectorConversion tests.
>  - 8268293: VectorAPI cast operation on mask and shuffle is broken

Thanks for your patience with my review comments. This is looking much better. Just a few minor comments.

test/jdk/jdk/incubator/vector/AbstractVectorConversionTest.java line 253:

> 251:                 for (Class<?> dstE : List.of(byte.class, short.class, int.class, long.class, float.class, double.class)) {
> 252:                     VectorSpecies<?> dst = VectorSpecies.of(dstE, dstShape);
> 253:                     if (legal) {

I think we just need to do `xnor` on `legal` and `dst.length() == src.length()`, we could simplify with:

// add if legal and equal lengths, or illegal and unequal lengths 
if (legal == (dst.length() == src.length())) {
    args.add(new Object[]{src, dst});
}

test/jdk/jdk/incubator/vector/AbstractVectorConversionTest.java line 524:

> 522: 
> 523:     static <E,F> void illegal_mask_cast_kernel(VectorSpecies<E> src, VectorSpecies<F> dst) {
> 524:         for(int i = 0; i < INVOC_COUNT; i++) {

We don't need the `INVOC_COUNT` loop for the illegal case, since the checks are performed in Java and not in the intrinsic.

-------------

PR: https://git.openjdk.java.net/jdk17/pull/39


More information about the hotspot-compiler-dev mailing list