RFR: 8367397: Improve naming and terminology in regmask.hpp and regmask.cpp [v4]
Emanuel Peter
epeter at openjdk.org
Fri Sep 12 12:33:44 UTC 2025
On Fri, 12 Sep 2025 11:36:27 GMT, Daniel Lundén <dlunden at openjdk.org> wrote:
>> Some names in `regmask.hpp` and `regmask.cpp` are unclear and should be improved.
>>
>> ### Changeset
>>
>> - Rename `RM_SIZE` to `RM_SIZE_IN_INTS` and `_RM_I` to `_RM_INT` to make it clear that these refer to integer-sized (32-bit) array elements.
>> - Rename `_RM_SIZE` to `_RM_SIZE_IN_WORDS` and `_RM_UP` to `_RM_WORD` to make it clear that these refer to machine-word-sized (32 or 64 bits depending on platform) array elements.
>> - Rename `_RM_MAX` to `_RM_WORD_MAX_INDEX` for clarity.
>> - Rename `is_AllStack` to `is_infinite` (and related resulting changes in comments and local variables). The old terminology "all-stack", referring to the infinite register mask bits, is misleading (as pointed out by @eme64 in https://github.com/openjdk/jdk/pull/20404#discussion_r2316234008). The reason is that the infinite bits do not represent *all* stack bits. Some stack bits are instead part of the non-infinite bits of the register mask.
>>
>> ### Testing
>>
>> - [GitHub Actions](https://github.com/dlunde/jdk/actions/runs/17638365968)
>> - `tier1` and HotSpot parts of `tier2` and `tier3` (and additional Oracle-internal testing) on Windows x64, Linux x64, Linux aarch64, macOS x64, and macOS aarch64.
>
> Daniel Lundén has updated the pull request incrementally with one additional commit since the last revision:
>
> Remove _LogWordBits
Changes requested by epeter (Reviewer).
src/hotspot/share/opto/regmask.hpp line 65:
> 63: LP64_ONLY(STATIC_ASSERT(is_aligned(RM_SIZE_IN_INTS, 2)));
> 64:
> 65: static const unsigned int _WordBitMask = BitsPerWord - 1U;
You could also remove the `_` here. I suppose we keep `CamelCase` here because to keep it parallel to `BitsPerWord`. What do you think @dean-long ?
src/hotspot/share/opto/regmask.hpp line 68:
> 66: static const unsigned int _RM_SIZE_IN_WORDS =
> 67: LP64_ONLY(RM_SIZE_IN_INTS >> 1) NOT_LP64(RM_SIZE_IN_INTS);
> 68: static const unsigned int _RM_WORD_MAX_INDEX = _RM_SIZE_IN_WORDS - 1U;
I would get rid of the `_` here. Constants should preferrably be `UPPER_CASE` (most of the time), and occasionally `CamelCase` where we are already doing it (only do it if needed for consistency). Underscore is only used for fields, as far as I know.
-------------
PR Review: https://git.openjdk.org/jdk/pull/27215#pullrequestreview-3216439510
PR Review Comment: https://git.openjdk.org/jdk/pull/27215#discussion_r2344087574
PR Review Comment: https://git.openjdk.org/jdk/pull/27215#discussion_r2344091072
More information about the hotspot-compiler-dev
mailing list