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