[vectorIntrinsics+fp16] RFR: 8302454: Improve VectorAPI fallback implementation for FP16 operations. [v2]

[Jatin Bhateja]

On Tue, 28 Feb 2023 12:56:27 GMT, Swati Sharma <duke at openjdk.org> wrote:

>> Hi All,
>> 
>> The patch contains the below changes:
>> - Remove redundant Halffloat object creation and rebase existing implementation using Float.float16ToFloat and floatToFloat16 APIs.
>> - Enable all the vector API tests for FP16 operations.
>> 
>> Please review and provide your feedback.
>> 
>> Thanks,
>> Swati
>
> Swati Sharma has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8302454: Resolved review comments.

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/AbstractMask.java line 388:

> 386:             if (vectorSpecies().elementType() == Halffloat.class) {
> 387:                 badMask2 =
> 388:                     iota.compare(LT, iota.broadcast(Float.floatToFloat16((float)firstGoodIndex)));

There is a flavor of  compare API which compares against scalar value, it can be used directly.

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Halffloat.java line 27:

> 25: package jdk.incubator.vector;
> 26: 
> 27: //import jdk.internal.vm.annotation.IntrinsicCandidate;

You can remove this commented import.

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/HalffloatVector.java line 2067:

> 2065:                 HalffloatVector iota = s.iota();
> 2066:                 short sc = (short) scale_;
> 2067:                 return v.add(sc == 1 ? iota : iota.mul(Halffloat.valueOf((float) sc)));

You can directly use Float.floatTofloat16( (float) sc), scale is a non NaN value.

test/jdk/jdk/incubator/vector/Float256VectorTests.java line 995:

> 993:             withToString("float[-i * 5]", (int s) -> {
> 994:                 return fill(s * BUFFER_REPS,
> 995:                             i -> valueOf(-i * 5));

Can you rename valueOf to genValue, by conventions valueOf is associated with Boxes.

test/jdk/jdk/incubator/vector/templates/Perf-Scalar-header.template line 76:

> 74:         cs = fill(i -> Halffloat.valueOf(i+5));
> 75:         rs = fill(i -> Halffloat.valueOf(0));
> 76: #else[Halffloat]

Specialization can be moved out.

test/jdk/jdk/incubator/vector/templates/Perf-header.template line 110:

> 108:         b = fill(i -> Halffloat.valueOf(i+1));
> 109:         c = fill(i -> Halffloat.valueOf(i+5));
> 110:         r = fill(i -> Halffloat.valueOf(0));

Specialization can be moved out.

test/jdk/jdk/incubator/vector/templates/Unit-header.template line 1706:

> 1704:         int scale = 2;
> 1705:         Class<?> ETYPE = $type$.class;
> 1706:         if (ETYPE == double.class || ETYPE == long.class)

Need refactoring here too, why do we need ETYPE double/long check here to.

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

PR: https://git.openjdk.org/panama-vector/pull/211