RFR: 8308469: [PPC64] Implement alternative fast-locking scheme

Richard Reingruber rrich at openjdk.org
Thu May 25 15:54:07 UTC 2023


On Sat, 20 May 2023 15:32:29 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

> New alternative fast-locking scheme for PPC64. Mostly implemented like on other platforms.
> Differences (also explained by comments in code):
> - Not using C2HandleAnonOMOwnerStub because the C2 code is reused for native wrappers.
> - Implemented a helper function `MacroAssembler::atomically_flip_locked_state` which makes it much easier to implement fast_lock/unlock for PPC64 (mainly because of register constraints in C1).
> - Using acquire/release barriers only for locking/unlocking.
> 
> I have changed the C2 code to use ConditionRegister CR0 which fits better to the new locking code. Therefore, I have adapted the other modes to work with that, too.
> Note that we don't support RTM with new locking modes. That feature will probably get removed in a future JDK version. (Already unsupported with Power10.)

Hi Martin,

thanks for doing the port. Just a few remarks and questions.

Cheers, Richard.

src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp line 1101:

> 1099:       // must handle it.
> 1100:       Register tmp = current_header;
> 1101:       // First check for lock-stack underflow.

I wonder why this is not done on x86? It's better to have that check.

src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp line 1111:

> 1109:       bne(CCR0, slow_case);
> 1110: 
> 1111:       ld(displaced_header, oopDesc::mark_offset_in_bytes(), object);

Maybe rename `displaced_header` to just `header`?

src/hotspot/cpu/ppc/macroAssembler_ppc.cpp line 2707:

> 2705:     }
> 2706:     beq(CCR0, success);
> 2707:     b(failure);

Nice improvements!

src/hotspot/cpu/ppc/macroAssembler_ppc.cpp line 2854:

> 2852:     andi_(R0, temp, ObjectMonitor::ANONYMOUS_OWNER);
> 2853:     bne(CCR0, anonymous);
> 2854:   }

This is surly very rare. Can't we take the slow path, i.e. branch to `failure`?

src/hotspot/cpu/ppc/ppc.ad line 12142:

> 12140: // inlined locking and unlocking
> 12141: 
> 12142: instruct cmpFastLock(flagsRegCR0 crx, iRegPdst oop, iRegPdst box, iRegPdst tmp1, iRegPdst tmp2) %{

I'm a little concerned about the usage of CR0. First it's not always clear if CR0 is altered when using the MacroAssembler, especially if not hacking ppc assembler on a daily basis. And also there might be a few instruction forms in the AD files that haven't got an effect defined for CR0 even though they alter it.

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

PR Review: https://git.openjdk.org/jdk/pull/14069#pullrequestreview-1441892063
PR Review Comment: https://git.openjdk.org/jdk/pull/14069#discussion_r1205555747
PR Review Comment: https://git.openjdk.org/jdk/pull/14069#discussion_r1205566598
PR Review Comment: https://git.openjdk.org/jdk/pull/14069#discussion_r1204163746
PR Review Comment: https://git.openjdk.org/jdk/pull/14069#discussion_r1205708168
PR Review Comment: https://git.openjdk.org/jdk/pull/14069#discussion_r1204455494


More information about the hotspot-dev mailing list