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

Roberto Castañeda Lozano rcastanedalo at openjdk.org
Thu Jan 11 12:44:24 UTC 2024


On Thu, 11 Jan 2024 10:19:12 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)
> - Add a regression test.
> 
> Testing:
> - [GitHub Actions](https://github.com/dlunde/jdk/actions/runs/7446998688)
> - tier1, tier2, tier3, tier4, and tier5 on windows-x64, linux-x64, linux-aarch64, macosx-x64, and macosx-aarch64

Changes requested by rcastanedalo (Reviewer).

src/hotspot/share/opto/locknode.cpp line 47:

> 45:   init_flags(Flag_rematerialize);
> 46:   OptoReg::Name reg = OptoReg::stack2reg(_slot);
> 47:   if (!RegMask::can_represent_arg(reg)) {

I am not very familiar with this code, but would it be possible to use `!RegMask::can_represent(reg)` instead of `!RegMask::can_represent_arg(reg)` here? Or is it necessary to use the latter (which is stricter) for correctness?

test/hotspot/jtreg/compiler/c2/TestNestedSynchronize.java line 27:

> 25:  * @test
> 26:  * @bug 8322996
> 27:  * @requires vm.debug

I suggest removing this line for better test coverage, the test does not really require debug mode.

test/hotspot/jtreg/compiler/c2/TestNestedSynchronize.java line 32:

> 30:  *
> 31:  * @run main/othervm -XX:CompileCommand=compileonly,compiler.c2.TestNestedSynchronize::test
> 32:  *                   -XX:-TieredCompilation -Xcomp

No need to use `-XX:-TieredCompilation` here (already in `-Xcomp` mode).

test/hotspot/jtreg/compiler/c2/TestNestedSynchronize.java line 36:

> 34:  */
> 35: 
> 36: package compiler.c2;

The test case might fit better under `test/hotspot/jtreg/compiler/locks`.

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

PR Review: https://git.openjdk.org/jdk/pull/17370#pullrequestreview-1815464703
PR Review Comment: https://git.openjdk.org/jdk/pull/17370#discussion_r1448790888
PR Review Comment: https://git.openjdk.org/jdk/pull/17370#discussion_r1448797933
PR Review Comment: https://git.openjdk.org/jdk/pull/17370#discussion_r1448798283
PR Review Comment: https://git.openjdk.org/jdk/pull/17370#discussion_r1448801274


More information about the hotspot-compiler-dev mailing list