RFR: 8317507: C2 compilation fails with "Exceeded _node_regs array" [v2]

[Aleksey Shipilev]

On Wed, 25 Oct 2023 14:20:00 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> This changeset avoids incrementing the global node index counter (`Compile::_unique`) when creating a CISC version node that replaces and reuses the index of the original node. This prevents out-of-bounds accesses to `PhaseRegAlloc::_node_regs` in programs with a high-density of CISC spill candidate nodes. Such programs can arise, for example, after extensive unrolling of reduction operations in x64 (see the [failure analysis](https://bugs.openjdk.org/browse/JDK-8317507) for further detail).
>> 
>> A more general fix to eliminate potential out-of-bounds accesses would be to transform `PhaseRegAlloc::_node_regs` into a growable array. This fix could also reduce memory overhead, as it would obviate the need for [over-allocating space for future nodes](https://github.com/openjdk/jdk/blob/fc29a2e152310ed81bd1bb23e6f17d02f055a454/src/hotspot/share/opto/regalloc.cpp#L113-L114). This is however a more intrusive, architecture-sensitive, and difficult-to-backport change (see prototype [here](https://github.com/openjdk/jdk/compare/master...robcasloz:jdk:JDK-8317507-exceeded-node-regs-growable-array)) that might be best addressed as a separate RFE. Note that the general fix would still benefit from the simpler one proposed in this changeset.
>> 
>> #### Testing
>> 
>> - tier1-5, stress test, fuzzing (windows-x64, linux-x64, linux-aarch64, macosx-x64, macosx-aarch64; release and debug mode).
>> - tier6-7 (linux-x64 only).
>
> Roberto Castañeda Lozano has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add hand-unrolled regression test

Looks good, with a minor nit in test.

test/hotspot/jtreg/compiler/regalloc/TestNodeRegArrayOverflow.java line 596:

> 594:             break;
> 595:         default:
> 596:             System.out.println("invalid mode");

Do `throw new IllegalArgumentException("Invalid mode: + args[0]);` here. The test should not silently pass on bad arguments.

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

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16306#pullrequestreview-1697544770
PR Review Comment: https://git.openjdk.org/jdk/pull/16306#discussion_r1371879854