On Wed, 25 Oct 2023 15:22:10 GMT, Ludovic Henry <luhenry at openjdk.org> wrote:
>> Feilong Jiang has updated the pull request incrementally with one additional commit since the last revision:
>>
>> adjust enum name style
>
> 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`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16362#discussion_r1373240773