RFR: 8322996: BoxLockNode creation fails with assert(reg < CHUNK_SIZE) failed: sanity [v5]

Vladimir Kozlov kvn at openjdk.org
Thu Jan 25 03:51:27 UTC 2024


On Wed, 24 Jan 2024 11:18:56 GMT, Daniel Lundén <dlunden at openjdk.org> wrote:

>> This changeset fixes an issue where deeply nested synchronized statements triggered an assert in C2.
>> 
>> Changes:
>> - Bail out on compilation when we create a `BoxLockNode` with a slot index that cannot fit in a `RegMask`. This is similar to how we handle the case when we do not have space to represent arguments in [`opto/matcher.cpp`](https://github.com/openjdk/jdk/blob/58b01dce054c50bcb5a28aad4c1b574acaa90f6d/src/hotspot/share/opto/matcher.cpp#L314-L318)
>> - Generalize `RegMask::can_represent` to take an additional and optional size argument to facilitate reuse. The default size value, 1, corresponds to the previous functionality. Rewrite `can_represent_arg` to directly call `can_represent(reg, SlotsPerVecZ)`.
>> - Add a regression test.
>> 
>> Testing:
>> - [GitHub Actions](https://github.com/dlunde/jdk/actions/runs/7612890820)
>> - tier1 to tier5 on windows-x64, linux-x64, linux-aarch64, macosx-x64, and macosx-aarch64.
>> - The new regression test in all tier1 to tier10 contexts on windows-x64, linux-x64, linux-aarch64, macosx-x64, and macosx-aarch64.
>
> Daniel Lundén has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix incorrect package name

Looks good. I have one question.

src/hotspot/share/opto/graphKit.cpp line 3473:

> 3471:   Node* box = _gvn.transform(new BoxLockNode(next_monitor()));
> 3472:   // Check for bailout after new BoxLockNode
> 3473:   if (failing()) { return nullptr; }

Do all callers of `shared_lock()` checks for `failing()` or returned `nullptr`?

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

PR Review: https://git.openjdk.org/jdk/pull/17370#pullrequestreview-1842788723
PR Review Comment: https://git.openjdk.org/jdk/pull/17370#discussion_r1465809098


More information about the hotspot-compiler-dev mailing list