On Fri, 12 Jul 2024 09:40:45 GMT, Roman Kennke <[email protected]> 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
