RFR: 8365911: AArch64: Fix encoding error in sve_cpy for negative floats [v2]

[Andrew Haley]

On Wed, 3 Sep 2025 07:22:27 GMT, erifan <duke at openjdk.org> wrote:

>> The sve_cpy instruction is not correctly implemented for negative floating-point values. The issues include:
>> 
>> 1. When a negative floating-point number (e.g. `-1.0`) is passed, the `checked_cast<int8_t>(pack(d))` check fails. For example, assume `d = -1.0`:
>> - `pack(-1.0)` returns an unsigned int with the 7th bit set, i.e., `0xf0`.
>> - `checked_cast<int8_t>(0xf0)` casts `0xf0` to an int8_t value, which is `-16`.
>> - Casting this int8_t `-16` back to unsigned int results in `0xfffffff0`.
>> - The check compares `0xf0` to `0xfffffff0`, which obviously fails.
>> 
>> 2. Additionally, the encoding of the negative floating-point number is incorrect:
>> - The imm8 field can fall outside the valid range of **[-128, 127]**.
>> - Bit **13** should be encoded as **0** for floating-point numbers.
>> 
>> This PR fixes these issues and renames floating-point `sve_cpy` as `sve_fcpy`.
>> 
>> Some test cases are added to aarch64-asmtest.py, and all tests passed.
>
> erifan 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:
> 
>  - Don't rename sve_cpy as sve_fcpy
>  - Merge branch 'master' into JDK-8365911
>  - 8365911: AArch64: Fix encoding error in sve_cpy for negative floats
>    
>    The sve_cpy instruction is not correctly implemented for negative
>    floating-point values. The issues include:
>    
>    1. When a negative floating-point number (e.g. `-1.0`) is passed, the
>    `checked_cast<int8_t>(pack(d))` check fails. For example, assume `d = -1.0`:
>    - `pack(-1.0)` returns an unsigned int with the 7th bit set, i.e., `0xf0`.
>    - `checked_cast<int8_t>(0xf0)` casts `0xf0` to an int8_t value, which is `-16`.
>    - Casting this int8_t `-16` back to unsigned int results in `0xfffffff0`.
>    - The check compares `0xf0` to `0xfffffff0`, which obviously fails.
>    
>    2. Additionally, the encoding of the negative floating-point number is incorrect:
>    - The imm8 field can fall outside the valid range of **[-128, 127]**.
>    - Bit **13** should be encoded as **0** for floating-point numbers.
>    
>    This PR fixes these issues and renames floating-point `sve_cpy` as `sve_fcpy`.
>    
>    Some test cases are added to aarch64-asmtest.py, and all tests passed.

This looks good, modulo the minor style fixes.

src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 3819:

> 3817:     if (isFloat) {
> 3818:       assert(T != B, "invalid size");
> 3819:       assert((imm8 >> 8) == 0, "invalid immediate");

Suggestion:

      assert((imm8 & 0xff) == 0, "invalid immediate");

To match line 3819.

src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 3831:

> 3829:     int m = isMerge ? 1 : 0;
> 3830:     f(0b00000101, 31, 24), f(T, 23, 22), f(0b01, 21, 20);
> 3831:     prf(Pg, 16), f(isFloat ? 1 : 0, 15), f(m, 14), f(sh, 13), f(imm8&0xff, 12, 5), rf(Zd, 0);

Suggestion:

    prf(Pg, 16), f(isFloat ? 1 : 0, 15), f(m, 14), f(sh, 13), f(imm8 & 0xff, 12, 5), rf(Zd, 0);

General HotSpot style.

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

PR Review: https://git.openjdk.org/jdk/pull/26951#pullrequestreview-3179466006
PR Review Comment: https://git.openjdk.org/jdk/pull/26951#discussion_r2318148242
PR Review Comment: https://git.openjdk.org/jdk/pull/26951#discussion_r2318149316