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

Roberto Castañeda Lozano rcastanedalo at openjdk.org
Wed Jan 24 09:39:28 UTC 2024


On Tue, 23 Jan 2024 10:14:00 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 two additional commits since the last revision:
> 
>  - Update missed copyright
>  - Refactor

The fix itself looks good to me.
Would it make sense, for better coverage, to add a couple of additional test cases that exercise the boundaries of the condition that is tested? E.g. one with one `synchronized` statement less than the current one and one with one  `synchronized` statement more.

test/hotspot/jtreg/compiler/locks/TestNestedSynchronize.java line 34:

> 32:  */
> 33: 
> 34: package compiler.c2;

Update package name to `compiler.locks`.

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

Changes requested by rcastanedalo (Reviewer).

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


More information about the hotspot-compiler-dev mailing list