RFR: 8325467: Support methods with many arguments in C2 [v27]

Emanuel Peter epeter at openjdk.org
Tue Sep 16 11:07:57 UTC 2025


On Tue, 16 Sep 2025 10:52:53 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> 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
>
> src/hotspot/share/opto/regmask.hpp line 267:
> 
>> 265: 
>> 266:   // Where to extend the register mask
>> 267:   Arena* _arena;
> 
> Usually, we try to keep all fields at the top.

Just to keep the overview.

> src/hotspot/share/opto/regmask.hpp line 464:
> 
>> 462:     copy(rm);
>> 463:     return *this;
>> 464:   }
> 
> You could also delete this one, and use the `copy` explicitly at the use site. That would make the allocations a bit more explicit. What do you think?
> Whenever possible, it is nice to be able to declare a type `NONCOPYABLE`. Especially if it does allocations where copy is non-trivial.

You already removed some assignments, like this one, which is good:
https://github.com/openjdk/jdk/pull/20404/files#diff-344e52fd6be79f1d97a33d7ebbf131148df90bb52e3b33952340e8d37a3849d8L1501-R1512

Generally, there are a lot of constructors here. All of them are public, none explicit. Maybe that is just how it has to be, but maybe you can simplify a little.

> src/hotspot/share/opto/regmask.hpp line 659:
> 
>> 657: 
>> 658:   // Fill a register mask with 1's starting from the given register.
>> 659:   void Set_All_From(OptoReg::Name reg) {
> 
> Oh boy, we have a mixture of `lower_case` and `Strange_Case` method names.
> We missed those in the renaming RFE :/

Or is there a particular logic behind it?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2351970696
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2351760214
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2351870907


More information about the hotspot-compiler-dev mailing list