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