RFR: 8363943: ARM32: Represent Registers as values [v2]

Aleksey Shipilev shade at openjdk.org
Wed Nov 12 18:39:39 UTC 2025


On Tue, 11 Nov 2025 22:50:44 GMT, Ivan <duke at openjdk.org> wrote:

>> 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`.
>
> The change is consistent with previous version. Previously the value was calculated in `src/hotspot/cpu/arm/register_arm.hpp` inside ConcreteRegisterImpl class like this:
> 
> #ifdef COMPILER2
>     log_bytes_per_fpr  = 2, // quad vectors
> #else
>     log_bytes_per_fpr  = 2, // double vectors
> #endif
>     ...
>     log_vmregs_per_fpr = log_bytes_per_fpr - LogBytesPerInt,
>     ...
>     vmregs_per_fpr = 1 << log_vmregs_per_fpr,
> 
> `LogBytesPerInt` in `globalDefinitions.hpp` is 2, so the `vmregs_per_fpr` always evaluated to 1.

OK, thanks.

>> 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.
>
> At the top of these definitions there is a comment
> 
> /*
>  * S1-S6 are named with "_reg" suffix to avoid conflict with
>  * constants defined in sharedRuntimeTrig.cpp
>  */
>  ```
>  And the definitions from `sharedRuntimeTrig.cpp` are still there
>  ```
>  static const double
> S1  = -1.66666666666666324348e-01, /* 0xBFC55555, 0x55555549 */
> S2  =  8.33333333332248946124e-03, /* 0x3F811111, 0x1110F8A6 */
> S3  = -1.98412698298579493134e-04, /* 0xBF2A01A0, 0x19C161D5 */
> S4  =  2.75573137070700676789e-06, /* 0x3EC71DE3, 0x57B1FE7D */
> S5  = -2.50507602534068634195e-08, /* 0xBE5AE5E6, 0x8A2B9CEB */
> S6  =  1.58969099521155010221e-10; /* 0x3DE5D93A, 0x5ACFD57C */
>  ```
>  
>  Should I ignore it and change `S${X}_reg` to `S${X}`? Or I shall avoid the conflict some other way?

Ooooof. Looks weird that the _common_ register code has to yield to `sharedRuntimeTrig.cpp`! But yeah, if it is inconvenient to do here, file a separate cleanup for it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26525#discussion_r2519388128
PR Review Comment: https://git.openjdk.org/jdk/pull/26525#discussion_r2519386467


More information about the hotspot-dev mailing list