RFR: 8350182: [s390x] Relativize locals in interpreter frames

Andrew Haley aph at openjdk.org
Tue Feb 18 11:55:19 UTC 2025


On Tue, 18 Feb 2025 11:37:27 GMT, Amit Kumar <amitkumar at openjdk.org> wrote:

>> src/hotspot/cpu/s390/frame_s390.hpp line 336:
>> 
>>> 334: #define _z_ijava_idx(_component) \
>>> 335:         (_z_ijava_state_neg(_component) >> LogBytesPerWord)
>>> 336: 
>> 
>> Should this be a function?
>> Also, names starting with `_` aren't common in HotSpot code, except for fields in C++ objects.
>
>> Should this be a function?
> 
> This is similar to [ppc ijava_idx](https://github.com/openjdk/jdk/blob/885be2efa6b1359a7c7ab36882e19a7eaba77fb3/src/hotspot/cpu/ppc/frame_ppc.hpp#L283C1-L285C58)
> 
>> Also, names starting with _ aren't common in HotSpot code, except for fields in C++ objects.
> 
> I did it for keeping the parity with: 
> 
> #define _z_ijava_state_neg(_component) \
>          (int) (-frame::z_ijava_state_size + offset_of(frame::z_ijava_state, _component))
> 
> Do you want me to revert it ?

There is no good reason to use a macro here. If a function can be a function, and this one can, let it be one. However, there is no reason to change anything else. Leave that for a "macros to functions" patch some other day.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23660#discussion_r1959589863


More information about the hotspot-dev mailing list