On Mon, 30 Mar 2026 16:07:26 GMT, Andrew Haley <[email protected]> wrote:
>> src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp line 1316:
>>
>>> 1314: = __ form_address(rscratch2, mdo,
>>> 1315: md->byte_offset_of_slot(data,
>>> DataLayout::flags_offset()),
>>> 1316: LogBytesPerWord);
>>
>> Oh, OK, cute. So this encodes the address more efficiently, knowing the
>> address is in the units of heap words? Can be upstreamed separately then?
>
> Seems like make work, but I guess so.
I agree this and the similar change below would be better factored out -- makes
the rest of this patch simpler for maintainers to understand.
>> src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp line 1987:
>>
>>> 1985:
>>> 1986: void LIR_Assembler::align_call(LIR_Code code) {
>>> 1987: __ save_profile_rng();
>>
>> Looks misplaced. Do you want to do it in `LIR_Assembler::call`, or something
>> gets in the way?
>
> Something does indeed. The size of the call is known and fixed, so we need to
> get anything variably sized in first.
When you say the call size is fixed what exactly do you mean? Does that include
the post call nop? the restore profile code?
It works hiding the save inside `align_call` (and likewise hiding the restore
in `call` and `ic_call`) for the arches you have currently implemented this for
and it will probably extend ok to riscv. But it doesn't look like it is as good
a fit for s390 where call alignment is handled differently.
Even if it can be made to work on s390 this still hides an important step
(well, actually both the save and restore) in an innocuous wrapper and that
smells bad to me. Can you not declare generic save and restore methods that
have to be called explicitly at places where a call is (aligned and) planted --
whether in generic or arch-specific code and then allow them to be implemented
in each arch? Decoupling these operations from the `align_call` or `call`
methods does risk a save and restore being missed but it also makes it clearer
what is happening.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3015126854
PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3020836147
