RFR: 8367397: Improve naming and terminology in regmask.hpp and regmask.cpp [v5]

Emanuel Peter epeter at openjdk.org
Fri Sep 12 12:43:11 UTC 2025


On Fri, 12 Sep 2025 12:33:42 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:
> 
>   Rename constants

Nice, thanks for the improvements. I feel like the fog is lifting slowly from this code, and we can see the sunshine :sun_behind_large_cloud: -> :sun_behind_small_cloud: -> 🌞  :rofl:

src/hotspot/share/opto/chaitin.hpp line 51:

> 49: class LRG : public ResourceObj {
> 50: public:
> 51:   static const uint InfiniteStack_size = 0xFFFFF; // This mask size is used to tell that the mask of this LRG supports stack positions

We may want to prevent this from snowballing everywhere ... but this is also a constant and we probably want to call it `INFINITE_STACK_SIZE`, right?

src/hotspot/share/opto/regmask.cpp line 249:

> 247:     if (_rm_word[i]) {                // Found some bits
> 248:       // Convert to bit number, return hi bit in pair
> 249:       return OptoReg::Name((i<<LogBitsPerWord) + find_lowest_bit(_rm_word[i]) + (size - 1));

Suggestion:

      return OptoReg::Name((i << LogBitsPerWord) + find_lowest_bit(_rm_word[i]) + (size - 1));

Might as well fix code style while we touch it ;)

-------------

PR Review: https://git.openjdk.org/jdk/pull/27215#pullrequestreview-3216478619
PR Review Comment: https://git.openjdk.org/jdk/pull/27215#discussion_r2344114954
PR Review Comment: https://git.openjdk.org/jdk/pull/27215#discussion_r2344116691


More information about the hotspot-compiler-dev mailing list