RFR: 8288300: AArch64: Remove the assertion in fmovs/fmovd(FloatRegister, FloatRegister)
Hao Sun
haosun at openjdk.java.net
Wed Jun 15 06:41:39 UTC 2022
On Wed, 15 Jun 2022 06:20:12 GMT, Andrew Haley <aph at openjdk.org> wrote:
>> The assertion, i.e. src and dst must be different registers, was
>> introduced years ago. But I don't think it's needed.
>>
>> This limitation was added in [1]. Frankly speaking, I don't know the
>> reason. But I guess the assertion is probably used for debugging,
>> raising one warning of fmovs/fmovd usage in the scenario of moving
>> element at index zero from one **vector** register, to one float-point
>> scalar register. If the "src" vector register and the "dst" float-point
>> scalar register are the same one, it introduces a side-effect, i.e. the
>> higher bits are cleared to zeros[2].
>>
>> If so, I argue that
>> 1) the assembler should align with the ISA.
>> 2) compiler developers should be aware of the side-effect when they want
>> to use fmovs/fmovd, and they should guarantee "dst != src" if they like
>> to higher bits untouched, e.g., [3].
>>
>> Hence, I think we can remove this unnecessary assertion.
>>
>> [1] http://hg.openjdk.java.net/aarch64-port/jdk8/hotspot/rev/9baee4e65ac5
>> [2] https://developer.arm.com/documentation/ddi0596/2021-12/SIMD-FP-Instructions/FMOV--register---Floating-point-Move-register-without-conversion-?lang=en
>> [3] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/aarch64/aarch64_sve.ad#L4899
>
> src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 1935:
>
>> 1933: INSN(fmovs, 0b000, 0b00, 0b000000);
>> 1934: INSN(fabss, 0b000, 0b00, 0b000001);
>> 1935: INSN(fnegs, 0b000, 0b00, 0b000010);
>
> There's unnecessary whitespace here.
> Suggestion:
>
> INSN(fmovs, 0b000, 0b00, 0b000000);
> INSN(fabss, 0b000, 0b00, 0b000001);
> INSN(fnegs, 0b000, 0b00, 0b000010);
Thanks for your review.
It's a style issue. I added one whitespace for `fmov/fabs/fneg` and the later `fcvt` so as to align the arguments to those of `fsqrt`. I saw such a style in several other sites in this header.
If you don't like it, I can remove the added whitespace. But I guess you may also want me to remove the whitespace I added for the `fcvt` below.
-------------
PR: https://git.openjdk.org/jdk/pull/9163
More information about the hotspot-compiler-dev
mailing list