On Tue, 5 May 2026 18:13:01 GMT, Paul Sandoz <[email protected]> wrote:
>> Jatin Bhateja has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments resolutions
>
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16Vector.java
> line 198:
>
>> 196:
>> 197: /*package-private*/
>> 198: interface FSnOp {
>
> You replaced `U` with `S`, which removes the connection to a unary operation.
> Same for the methods. I gather this is only required for handling the unary
> sqrt operation. I am not sure we need them, if we do we should name them more
> clearly. I will leave a comment next to the unary sqrt operation.
I have modified the nomenclature to preserve the operation cardinality
connection, instead of sOp I am now using uRawOp. sOp was used at two places
one for numerically accurate Float16.sqrt operation and secondly in
rearrangeTemplate implementation, this was done to save redundant up casting of
bare short values returned by these APIs to float which will impact fallback
performance.
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16Vector.java
> line 803:
>
>> 801: v0.uOp(m, (i, a) -> (float) Math.log10(a));
>> 802: case VECTOR_OP_SQRT: return (v0, m) ->
>> 803: v0.sOp(m, (i, a) -> (short)
>> float16ToRawShortBits(Float16.sqrt(shortBitsToFloat16(a))));
>
> Can we reuse `uOp` and convert the result of `Float16.sqrt` to `float`
> instead?
We can but as mentioned above sOp is also used for rearrangeTemplate, should be
also make a change there ?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28002#discussion_r3194685165
PR Review Comment: https://git.openjdk.org/jdk/pull/28002#discussion_r3194685497
