RFR: 8325467: Support methods with many arguments in C2 [v27]
Emanuel Peter
epeter at openjdk.org
Tue Sep 16 12:49:00 UTC 2025
On Tue, 16 Sep 2025 08:52:57 GMT, Daniel Lundén <dlunden at openjdk.org> wrote:
>> If a method has a large number of parameters, we currently bail out from C2 compilation.
>>
>> ### Changeset
>>
>> Allowing C2 compilation of methods with a large number of parameters requires fundamental changes to the register mask data structure, used in many places in C2. In particular, register masks currently have a statically determined size and cannot represent arbitrary numbers of stack slots. This is needed if we want to compile methods with arbitrary numbers of parameters. Register mask operations are present in performance-sensitive parts of C2, which further complicates changes.
>>
>> Changes:
>> - Add functionality to dynamically grow/extend register masks. I experimented with a number of design choices to achieve this. To keep the common case (normal number of method parameters) quick and also to avoid more intrusive changes to the current `RegMask` interface, I decided to leave the "base" statically allocated memory for masks unchanged and only use dynamically allocated memory in the rare cases where it is needed.
>> - Generalize the "chunk"-logic from `PhaseChaitin::Select()` to allow arbitrary-sized chunks, and also move most of the logic into register mask methods to separate concerns and to make the `PhaseChaitin::Select()` code more readable.
>> - Remove all `can_represent` checks and bailouts.
>> - Performance tuning. A particularly important change is the early-exit optimization in `RegMask::overlap`, used in the performance-sensitive method `PhaseChaitin::interfere_with_live`.
>> - Add a new test case `TestManyMethodArguments.java` and extend an old test `TestNestedSynchronize.java`.
>>
>> ### Testing
>>
>> - [GitHub Actions](https://github.com/dlunde/jdk/actions/runs/10178060450)
>> - `tier1` to `tier4` (and additional Oracle-internal testing) on Windows x64, Linux x64, Linux aarch64, macOS x64, and macOS aarch64.
>> - Standard performance benchmarking. No observed conclusive overall performance degradation/improvement.
>> - Specific benchmarking of C2 compilation time. The changes increase C2 compilation time by, approximately and on average, 1% for methods that could also be compiled before this changeset (see the figure below). The reason for the degradation is further checks required in performance-sensitive code (in particular `PhaseChaitin::remove_bound_register_from_interfering_live_ranges`). I have tried optimizing in various ways, but changes I found that lead to improvement also lead to less readable code (and are, in my opinion, no...
>
> Daniel Lundén has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 39 commits:
>
> - Clarify comments in regmask.hpp
> - Merge remote-tracking branch 'upstream/master' into many-arguments-8325467+pr-updates
> - Address review comments (renaming on the way in a separate PR)
> - Update src/hotspot/share/opto/regmask.hpp
>
> Co-authored-by: Emanuel Peter <emanuel.peter at oracle.com>
> - Restore modified java/lang/invoke tests
> - Sort includes (new requirement)
> - Merge remote-tracking branch 'upstream/master' into many-arguments-8325467+pr-updates
> - Add clarifying comments at definitions of register mask sizes
> - Fix implicit zero and nullptr checks
> - Add deep copy comment
> - ... and 29 more: https://git.openjdk.org/jdk/compare/60930a3e...c1f41288
Alright, sprinted through the end.
I really appreciate that you added extensive `gtest`s, thanks for that 😊
And thanks for using the Template Framework, I'm curious to hear if you have any feedback on it :)
src/hotspot/share/opto/chaitin.cpp line 1656:
> 1654:
> 1655: // Check if a color is available and if so pick the color
> 1656: OptoReg::Name reg = choose_color(*lrg);
Accidental find: why is this assert commented out?
src/hotspot/share/opto/chaitin.cpp line 1663:
> 1661: if (!OptoReg::is_valid(reg) && is_infinite_stack) {
> 1662: // Bump register mask up to next stack chunk
> 1663: bool success = lrg->rollover();
Can you add a comment that explains what this does / means? Do we start spilling to the stack slots instead of using registers?
src/hotspot/share/opto/regmask.hpp line 241:
> 239: // \_______________________________________________________________________________/
> 240: // |
> 241: // _rm_size_in_words=_offset=5
Can you please add some concise comment why we need `rollover`? Does that happen during register allocation, and if we have rollover then we start spilling instead of keeping values in registers?
src/hotspot/share/opto/regmask.hpp line 837:
> 835: // ----------------------------------------------------------------------
> 836: // The methods below are only for testing purposes (see test_regmask.cpp)
> 837: // ----------------------------------------------------------------------
I wonder if it could be solved with `friend` instead, so it does not have to be public and get accidentally used somehow.
Or maybe some `gtest_` prefix? Not sure.
test/hotspot/jtreg/compiler/arguments/TestMethodArguments.java line 51:
> 49: static final int INPUT_SIZE = 100;
> 50:
> 51: public static Template.ZeroArgs generateTest(PrimitiveType t, int numberOfArguments) {
You should write out `type` instead of `t`, would make it consistent with your `let` below.
test/hotspot/jtreg/compiler/arguments/TestMethodArguments.java line 120:
> 118: Template.let("classpath", comp.getEscapedClassPathOfCompiledClasses()),
> 119: """
> 120: import java.util.Arrays;
Personally, I would not indent this deeply. I know that the generated code will not have proper indentation, but that's no so bad. Readability of the Templates is more important I think. Subjective though.
test/hotspot/jtreg/compiler/arguments/TestMethodArguments.java line 146:
> 144: }
> 145: return array;
> 146: }
Seems like we need to add some convenience "fill" methods to the template library. We'll get there eventually, just keep this for now.
-------------
Changes requested by epeter (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/20404#pullrequestreview-3229588008
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2352217532
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2352235070
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2352231339
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2352309374
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2352361517
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2352382893
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2352378439
More information about the hotspot-compiler-dev
mailing list