Integrated: 8324969: C2: prevent elimination of unbalanced coarsened locking regions

[Vladimir Kozlov]

On Sat, 3 Feb 2024 21:56:25 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

> The issue is that when we do nested locks elimination we don't check that synchronized region has both `Lock` and `Unlock` nodes.
> Which can happen if locks coarsening optimization eliminated pair of Unlock/Lock nodes from adjacent locking regions before we check for nested locks.
> 
> Consider this code (all locks/unlocks use the same object):
> 
>     lock1(obj) { // outer synchronized region
>          lock2(obj) {
>              // nested synchronized region 1
>          } unlock2(obj);
>          lock3(obj) {
>              // nested synchronized region 2
>          } unlock3(obj);
>     } unlock1(obj);
> 
> If `lock3` directly follows `unlock2` (no branches or safepoints) locks coarsening optimization will remove them:
> 
> 
>     lock1(obj) { // outer synchronized region
>          lock2(obj) {
>              // nested synchronized region 1
>          };
>          {
>              // nested synchronized region 2
>          } unlock3(obj);
>     } unlock1(obj);
> 
> 
> Nested locks elimination code checks only `Lock` node in one region to find if it is nested (inside other lock region which use the same object) and then eliminate it. So we end up with not eliminated `Unlock` node in second nested region.
> 
> Why we don't hit this issue before? Normally nested locks elimination executed first and only then we do locks coarsening elimination. In the example above we eliminate all nested `Lock` and `Unlock` nodes, leaving only outer `Lock` and `Unlock`.
> 
> The additional factors which leads to the failure is fully unrolled loop around nested sync regions and some allocation to trigger Escape Analysis:
> 
> 
>     lock1(obj) { // outer synchronized region
>          Test var = new Test(); // Triggers EA
>          for (I = 0; I < 3; i++) { // small iteration number to fully unroll
>              lock2(obj) {
>                  // nested synchronized region 1
>              } unlock2(obj);
>              lock3(obj) {
>                  // nested synchronized region 2
>              } unlock3(obj);
>          }
>     } unlock1(obj);
> 
> Before executing Escape Analysis we do loops optimization to simplify graph: [compile.cpp#L2332](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/compile.cpp#L2332)
> We also allow to fully unroll short loops (LoopOptsMaxUnroll) to remove merges from graph. It helps EA eliminate allocations.
> Such unrolling creates several `Lock` and `Unlock` nodes per synchronized region. But nested locks elimination look for region with only one unique `Lock` node: [callnode.cpp#L2117](https://github.com/open...

This pull request has now been integrated.

Changeset: b938a5c9
Author:    Vladimir Kozlov <kvn at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/b938a5c9edd53821a52b43a8e342b76adb341a3f
Stats:     273 lines in 9 files changed: 255 ins; 2 del; 16 mod

8324969: C2: prevent elimination of unbalanced coarsened locking regions

Reviewed-by: epeter, vlivanov, dlong

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

PR: https://git.openjdk.org/jdk/pull/17697