RFR: 8315884: New Object to ObjectMonitor mapping [v6]

Andrew Haley aph at openjdk.org
Fri Jul 12 12:08:55 UTC 2024


On Fri, 12 Jul 2024 09:40:45 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> Axel Boldt-Christmas has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update arguments.cpp
>
> src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 343:
> 
>> 341:     const Register t3_owner = t3;
>> 342:     const ByteSize monitor_tag = in_ByteSize(UseObjectMonitorTable ? 0 : checked_cast<int>(markWord::monitor_value));
>> 343:     const Address owner_address{t1_monitor, ObjectMonitor::owner_offset() - monitor_tag};
> 
> That may be just me, but I found that syntax weird. I first needed to look-up what the {}-initializer actually means. Hiccups like this reduce readability, IMO. I'd prefer the normal ()-init for the Address like we seem to do everywhere else.

I agree with @rkennke . When we wrote the AArch64 MacroAssembler we were concentrating on readability and familiarity, and this separate declaration and use, with unusual syntax, IMO makes life harder for the reader.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1675751592


More information about the serviceability-dev mailing list