On Thu, 9 Jan 2025 19:22:35 GMT, Paul Sandoz <psan...@openjdk.org> wrote:
>> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Float16.java >> line 1434: >> >>> 1432: return float16ToRawShortBits(valueOf(product + >>> float16ToFloat(f16c))); >>> 1433: }); >>> 1434: return shortBitsToFloat16(res); >> >> I don't understand what is happening here. But I leave this to @PaulSandoz >> to review > > Uncertain on what bits, but i am guessing it's mostly related to the fallback > code in the lambda. To avoid the intrinsics operating on Float16 instances we > instead "unpack" the carrier (16bits) values and pass those as arguments to > the intrinsic. The fallback (when intrinsification is not supported) also > accepts those carrier values as arguments and we convert the carriers to > floats, operate on then, convert to the carrier, and then back to float16 on > the result. > > The code in the lambda could potentially be simplified if `Float16Math.fma` > accepted six arguments the first three being the carrier values used by the > intrinsic, and the subsequent three being the float16 values used by the > fallback. Then we could express the code in the original source in the > lambda. I believe when intrinsified there would be no penalty for those extra > arguments. Hi @PaulSandoz , In the current scheme we are passing unboxed carriers to intrinsic entry point, in the fallback implementation carrier type is first converted to floating point value using Float.float16ToFloat API which expects to receive a short type argument, after the operation we again convert float value to carrier type (short) using Float.floatToFloat16 API which expects a float argument, thus our intent here is to perform unboxing and boxing outside the intrinsic thereby avoiding all complexities around boxing by compiler. Even if we pass 3 additional parameters we still need to use Float16.floatValue which invokes Float.float16ToFloat underneath, thus this minor modification on Java side is on account of optimizing the intrinsic interface. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22754#discussion_r1912858286
