RFR: 8316746: Top of lock-stack does not match the unlocked object [v5]

[Lutz Schmidt]

On Thu, 2 Nov 2023 17:01:40 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> It took me quite a long journey to figure out that C2 OSR goes wrong only in the test "vmTestbase/nsk/jdi/StepEvent" because the interpreter fills the slots in a different order as expected by C2. (Interpreter and C1 don't care about it.)
>> I've reimplemented the search loop in `monitorenter`, improved comments and cleaned up the related code a bit. The test is passing with this change.
>
> Martin Doerr has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Improve comment.

Changes look good to me. 
I added one minor optimization suggestion you may want to consider. It is not relevant for approval.

src/hotspot/cpu/ppc/templateTable_ppc_64.cpp line 4219:

> 4217:   __ beq(CCR0, Lallocate_new);
> 4218:   __ mr(Rcurrent_monitor, Rfree_slot);
> 4219:   __ b(Lfound);

Wouldn't it be nice to use `Rfree_slot` from here on and save the move to `Rcurrent_monitor` and the unconditional branch?

  __ cmpdi(CCR0, Rfree_slot, 0);
  __ bne(CCR0, Lfound);

src/hotspot/cpu/ppc/templateTable_ppc_64.cpp line 4225:

> 4223:   __ bind(Lallocate_new);
> 4224:   __ add_monitor_to_stack(false, Rscratch1, Rscratch2);
> 4225:   __ mr(Rcurrent_monitor, R26_monitor);

If you do the above optimization, this instruction would need to change to 
` __ mr(Rfree_slot, R26_monitor);`

src/hotspot/cpu/ppc/templateTable_ppc_64.cpp line 4236:

> 4234: 
> 4235:   __ std(Robj_to_lock, in_bytes(BasicObjectLock::obj_offset()), Rcurrent_monitor);
> 4236:   __ lock_object(Rcurrent_monitor, Robj_to_lock);

`Rcurrent_monitor` would need to be replaced by `Rfree_slot` in the above two lines as well.

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

Marked as reviewed by lucy (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16406#pullrequestreview-1720072074
PR Review Comment: https://git.openjdk.org/jdk/pull/16406#discussion_r1386453187
PR Review Comment: https://git.openjdk.org/jdk/pull/16406#discussion_r1386454777
PR Review Comment: https://git.openjdk.org/jdk/pull/16406#discussion_r1386458424