RFR: 8288300: AArch64: Remove the assertion in fmovs/fmovd(FloatRegister, FloatRegister)
Andrew Haley
aph at openjdk.java.net
Wed Jun 15 06:56:45 UTC 2022
On Wed, 15 Jun 2022 06:37:48 GMT, Hao Sun <haosun at openjdk.org> wrote:
>> 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.
I see. You're right, we're not consistent about this in the file.
There are advantages and disadvantages. On the one hand it's a little easier to read, but on the other it's more work to maintain and tends to get eroded over time.
Whitespace-only changes as part of a larger patch can be hard to review , because they ca hide the substantive changes.
Please take out the added whitespace in this patch, and submit just the substantive changes. I will think about what to do to make this file consistent.
-------------
PR: https://git.openjdk.org/jdk/pull/9163
More information about the hotspot-compiler-dev
mailing list