On Wed, 26 Aug 2026 06:03:36 GMT, Harshit Dhiman <[email protected]> wrote:

>> src/hotspot/cpu/s390/gc/z/zBarrierSetAssembler_s390.cpp line 438:
>> 
>>> 436:     assert(zpointer != Z_ARG2, "or change argument setup");
>>> 437:     __ lgr_if_needed(Z_ARG2, addr);
>>> 438:     __ 
>>> call_VM_leaf(ZBarrierSetRuntime::load_barrier_on_oop_field_preloaded_addr(),
>>>  zpointer, Z_ARG2);
>> 
>> let call_VM_leaf handle the shuffling. 
>> Suggestion:
>> 
>>     __ 
>> call_VM_leaf(ZBarrierSetRuntime::load_barrier_on_oop_field_preloaded_addr(), 
>> zpointer, addr);
>
> We can't do this here as `zpointer` and `addr` here is `R1` and `R2` 
> respectively. Here we are loading `addr` into `Z_ARG2` first and then 
> `call_VM_leaf` will load `zpointer` into `Z_ARG1`.
> But by default `call_VM_leaf` will try to put `zpointer` into `Z_ARG1` 
> overwriting `addr`. (We will just hit a assert failure there)
> 
> I can do it in a separate PR by changing the shuffling to a more robust 
> implementation something like we have done in `ZSetupArguments` below.

But I can just let call_VM_leaf shuffle the arguments at other places like in 
load_at.

>> src/hotspot/cpu/s390/gc/z/zBarrierSetAssembler_s390.cpp line 516:
>> 
>>> 514:   const Register zpointer = Z_R1;
>>> 515:   Label done, loop, load_bad, load_good, store_bad, store_good;
>>> 516:   __ z_slag(Z_R0, Z_ARG3, 3);
>> 
>> should we zero-extend Z_ARG3 here ? because I am not sure if there exist a 
>> guarantee that upper half will be zero or not.
>
> It's not guaranteed that upper half will be zero. But in the implementation 
> for the non zGC version of this they are using an assert there.
> See `assert_positive_int` in 
> https://github.com/openjdk/jdk/blob/c436633bb9be5854bc8c7eacdde3faae28aef3a9/src/hotspot/cpu/s390/stubGenerator_s390.cpp#L832

We can just move the implementation of `assert_positive_int` to 
`macroAssembler_s390.cpp` and use it here as well.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/31984#discussion_r3860065379
PR Review Comment: https://git.openjdk.org/jdk/pull/31984#discussion_r3861861824

Reply via email to