RFR: 8363943: ARM32: Represent Registers as values

[Aleksey Shipilev]

On Tue, 29 Jul 2025 06:46:56 GMT, Ivan <duke at openjdk.org> wrote:

> Migrate away from pointer-based representation of Register values.
> 
> It improves compile-time checking by forbidding implicit conversions between integrals and pointers.
> 
> [JDK-8363943](https://bugs.openjdk.org/browse/JDK-8363943)

Good work, looks pretty reasonable. Questions/comments/nits:

src/hotspot/cpu/arm/register_arm.hpp line 86:

> 84:   enum {
> 85:     number_of_registers = 16,
> 86:     max_slots_per_register = 1 << (LogBytesPerWord - LogBytesPerInt) // LogBytesPerWord depends on _LP64

ARM32 is only 32-bit, so we can skip any _LP64-based computations, and just do the literal constant.

src/hotspot/cpu/arm/register_arm.hpp line 101:

> 99: 
> 100:     // testers
> 101:     bool is_valid() const {return 0 <= raw_encoding() && raw_encoding() < number_of_registers;}

Suggestion:

    // accessors and testers
    int raw_encoding() const { return this - first(); } 
    int encoding() const     { assert(is_valid(), "invalid register"); return raw_encoding(); }
    bool is_valid() const    { return 0 <= raw_encoding() && raw_encoding() < number_of_registers; }

src/hotspot/cpu/arm/register_arm.hpp line 152:

> 150: constexpr Register R9 = as_Register(9);
> 151: constexpr Register R10 = as_Register(10);
> 152: constexpr Register R11 = as_Register(11);

Indent these like:


constexpr Register R8  = as_Register( 8);
constexpr Register R9  = as_Register( 9);
constexpr Register R10 = as_Register(10);
constexpr Register R11 = as_Register(11);

src/hotspot/cpu/arm/register_arm.hpp line 187:

> 185:   enum {
> 186:     number_of_registers = NOT_COMPILER2(32) COMPILER2_PRESENT(64),
> 187:     max_slots_per_register = 1

Can you double-check it is really `1`? For GPRs, we have `max_slots_per_register` at effectively `2`.

src/hotspot/cpu/arm/register_arm.hpp line 202:

> 200: 
> 201:     // testers
> 202:     bool is_valid() const {return 0 <= raw_encoding() && raw_encoding() < number_of_registers;}

Suggestion:

    // accessors and testers
    int raw_encoding() const { return this - first(); }
    int encoding() const     { assert(is_valid(), "invalid register"); return raw_encoding(); }
    bool is_valid() const    { return 0 <= raw_encoding() && raw_encoding() < number_of_registers; }

src/hotspot/cpu/arm/register_arm.hpp line 264:

> 262: constexpr FloatRegister S4_reg = as_FloatRegister(4);
> 263: constexpr FloatRegister S5_reg = as_FloatRegister(5);
> 264: constexpr FloatRegister S6_reg = as_FloatRegister(6);

Take a chance on renaming these `S${X}_reg` to just `S${X}`? I spot-checked their usages, and there are only a few places that need adjustments.

src/hotspot/cpu/arm/register_arm.hpp line 265:

> 263: constexpr FloatRegister S5_reg = as_FloatRegister(5);
> 264: constexpr FloatRegister S6_reg = as_FloatRegister(6);
> 265: constexpr FloatRegister S7 = as_FloatRegister(7);

Also, indent this like:


constexpr FloatRegister S8  = as_FloatRegister( 8);
constexpr FloatRegister S9  = as_FloatRegister( 9);
constexpr FloatRegister S10 = as_FloatRegister(10);
constexpr FloatRegister S11 = as_FloatRegister(11);

src/hotspot/cpu/arm/register_arm.hpp line 427:

> 425: constexpr VFPSystemRegister FPSCR = as_VFPSystemRegister(  1);
> 426: constexpr VFPSystemRegister MVFR0 = as_VFPSystemRegister(0x6);
> 427: constexpr VFPSystemRegister MVFR1 = as_VFPSystemRegister(0x7);

You can use `VFPSystemRegister` enum values as arguments here, correct? Like:


constexpr VFPSystemRegister MVFR1 = as_VFPSystemRegister(VFPSystemRegister::MVFR1);

src/hotspot/cpu/arm/vmreg_arm.hpp line 52:

> 50:       return (value() % Register::max_slots_per_register == 0);
> 51:     } else if (is_FloatRegister()) {
> 52:       return true; // Single slot

I guess. But for safety, we can still do `% FloatRegister::max_slot_per_register == 0`, just in case we ever need to adjust it?

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

PR Review: https://git.openjdk.org/jdk/pull/26525#pullrequestreview-3442536328
PR Review Comment: https://git.openjdk.org/jdk/pull/26525#discussion_r2510005169
PR Review Comment: https://git.openjdk.org/jdk/pull/26525#discussion_r2510013454
PR Review Comment: https://git.openjdk.org/jdk/pull/26525#discussion_r2510042736
PR Review Comment: https://git.openjdk.org/jdk/pull/26525#discussion_r2510467475
PR Review Comment: https://git.openjdk.org/jdk/pull/26525#discussion_r2510025802
PR Review Comment: https://git.openjdk.org/jdk/pull/26525#discussion_r2510036921
PR Review Comment: https://git.openjdk.org/jdk/pull/26525#discussion_r2510039259
PR Review Comment: https://git.openjdk.org/jdk/pull/26525#discussion_r2510477885
PR Review Comment: https://git.openjdk.org/jdk/pull/26525#discussion_r2510485666