RFR: 8318827: RISC-V: Improve readability of fclass result testing [v2]
Vladimir Kempik
vkempik at openjdk.org
Thu Oct 26 15:12:34 UTC 2023
On Thu, 26 Oct 2023 14:05:58 GMT, Feilong Jiang <fjiang at openjdk.org> wrote:
>> src/hotspot/cpu/riscv/assembler_riscv.hpp line 1094:
>>
>>> 1092: FCLASS_NAN = FCLASS_SNAN | FCLASS_QNAN,
>>> 1093: FCLASS_FINITE = FCLASS_ZERO | FCLASS_SUBNORM | FCLASS_NORM,
>>> 1094: };
>>
>> We use lower-case and no prefix for similar enums (ex: https://github.com/openjdk/jdk/blob/069bb87693ec7944f08062e3a7e460653324e6cb/src/hotspot/cpu/riscv/assembler_riscv.hpp#L1119-L1145)
>>
>> Suggestion:
>>
>> enum fclass_mask {
>> minf = 1 << 0, // negative infinite
>> mnorm = 1 << 1, // negative normal number
>> msubnorm = 1 << 2, // negative subnormal number
>> mzero = 1 << 3, // negative zero
>> pzero = 1 << 4, // positive zero
>> psubnorm = 1 << 5, // positive subnormal number
>> pnorm = 1 << 6, // positive normal number
>> pinf = 1 << 7, // positive infinite
>> snan = 1 << 8, // signaling NaN
>> qnan = 1 << 9, // quiet NaN
>> zero = mzero | pzero,
>> subnorm = msubnorm | psubnorm,
>> norm = mnorm | pnorm,
>> inf = minf | pinf,
>> nan = snan | qnan,
>> finite = zero | subnorm | norm,
>> };
>
> Changed the enum to lowercase. But I think we should keep the `flcass_` prefix to make sure it‘s only used for `flcass`.
just use them as
Assembler::fclass_mask::msubnorm ( when enum itself is given the name) and it's pretty clear what type it describes
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16362#discussion_r1373336563
More information about the hotspot-dev
mailing list