On Fri, 19 Dec 2025 08:50:59 GMT, Xiaohong Gong <[email protected]> wrote:
>> Bhavana Kilambi 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 three additional
>> commits since the last revision:
>>
>> - Address review comments
>> - Merge 'master'
>> - 8366444: Add support for add/mul reduction operations for Float16
>>
>> This patch adds mid-end support for vectorized add/mul reduction
>> operations for half floats. It also includes backend aarch64 support for
>> these operations. Only vectorization support through autovectorization
>> is added as VectorAPI currently does not support Float16 vector species.
>>
>> Both add and mul reduction vectorized through autovectorization mandate
>> the implementation to be strictly ordered. The following is how each of
>> these reductions is implemented for different aarch64 targets -
>>
>> For AddReduction :
>> On Neon only targets (UseSVE = 0): Generates scalarized additions
>> using the scalar "fadd" instruction for both 8B and 16B vector lengths.
>> This is because Neon does not provide a direct instruction for computing
>> strictly ordered floating point add reduction.
>>
>> On SVE targets (UseSVE > 0): Generates the "fadda" instruction which
>> computes add reduction for floating point in strict order.
>>
>> For MulReduction :
>> Both Neon and SVE do not provide a direct instruction for computing
>> strictly ordered floating point multiply reduction. For vector lengths
>> of 8B and 16B, a scalarized sequence of scalar "fmul" instructions is
>> generated and multiply reduction for vector lengths > 16B is not
>> supported.
>>
>> Below is the performance of the two newly added microbenchmarks in
>> Float16OperationsBenchmark.java tested on three different aarch64
>> machines and with varying MaxVectorSize -
>>
>> Note: On all machines, the score (ops/ms) is compared with the master
>> branch without this patch which generates a sequence of loads ("ldrsh")
>> to load the FP16 value into an FPR and a scalar "fadd/fmul" to
>> add/multiply the loaded value to the running sum/product. The ratios
>> given below are the ratios between the throughput with this patch and
>> the throughput without this patch.
>> Ratio > 1 indicates the performance with this patch is better than the
>> master branch.
>>
>> N1 (UseSVE = 0, max vector length = 16B):
>> Benchmark vecto...
>
> src/hotspot/cpu/aarch64/aarch64_vector.ad line 3490:
>
>> 3488: %}
>> 3489: ins_pipe(pipe_slow);
>> 3490: %}
>
> Could you please float this rule above `reduce_addF_sve` and below
> `reduce_addHF`? Better to rename `reduce_addHF` to `reduce_addHF_neon` ?
Done
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27526#discussion_r2640448612
