RFR: 8310513: [s390x] Intrinsify recursive ObjectMonitor locking

Lutz Schmidt lucy at openjdk.org
Fri Apr 5 04:04:32 UTC 2024


On Fri, 23 Feb 2024 05:23:29 GMT, Amit Kumar <amitkumar at openjdk.org> wrote:

> s390 implementation of [JDK-8277180](https://bugs.openjdk.org/browse/JDK-8277180). PPC implementation for the same: https://github.com/openjdk/jdk/pull/7305
> 
> I had tested `tier1` on `fastdebug`, `release` vm. 
> 
> BenchMarking: 
> 
> 
> ./build/linux-s390x-server-release/jdk/bin/java -Xms4g -Xmx4g -jar dacapo-9.12-MR1-bach.jar h2 -s huge -t 1 -n 1
> 
> without patch: 
> ===== DaCapo 9.12-MR1 h2 PASSED in 223023 msec =====
> ===== DaCapo 9.12-MR1 h2 PASSED in 225686 msec =====
> ===== DaCapo 9.12-MR1 h2 PASSED in 219824 msec =====
> ===== DaCapo 9.12-MR1 h2 PASSED in 226719 msec =====
> 
> 
> 
> with patch: 
> ===== DaCapo 9.12-MR1 h2 PASSED in 167816 msec =====
> ===== DaCapo 9.12-MR1 h2 PASSED in 174368 msec =====
> ===== DaCapo 9.12-MR1 h2 PASSED in 170517 msec =====
> ===== DaCapo 9.12-MR1 h2 PASSED in 169349 msec =====

Changes requested by lucy (Reviewer).

src/hotspot/cpu/s390/macroAssembler_s390.cpp line 3280:

> 3278: 
> 3279:   // Current thread already owns the lock. Just increment recursion.
> 3280:   z_asi(Address(currentHeader, OM_OFFSET_NO_MONITOR_VALUE_TAG(recursions)), 1);

This add will compromise your CC setting. Recursive locking was successful, so you need to maintain an "EQUAL" condition code.

src/hotspot/cpu/s390/macroAssembler_s390.cpp line 3350:

> 3348:   // Recursive inflated unlock
> 3349:   z_asi(Address(currentHeader, OM_OFFSET_NO_MONITOR_VALUE_TAG(recursions)), -1);
> 3350:   z_cgr(currentHeader, currentHeader); // set the CC 1

Bad comment. 
This instruction sets the CC to 0b00 ("EQUAL"). That corresponds to a condition code mask value of 0x8.

src/hotspot/cpu/s390/macroAssembler_s390.cpp line 3351:

> 3349:   z_asi(Address(currentHeader, OM_OFFSET_NO_MONITOR_VALUE_TAG(recursions)), -1);
> 3350:   z_cgr(currentHeader, currentHeader); // set the CC 1
> 3351:   z_bre(done);

For clarity, I would prefer to use z_bru(done);

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

PR Review: https://git.openjdk.org/jdk/pull/17975#pullrequestreview-1900834942
PR Review Comment: https://git.openjdk.org/jdk/pull/17975#discussion_r1502587029
PR Review Comment: https://git.openjdk.org/jdk/pull/17975#discussion_r1502592243
PR Review Comment: https://git.openjdk.org/jdk/pull/17975#discussion_r1502593386


More information about the hotspot-dev mailing list