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

[Daniel Lundén]

On Tue, 16 Sep 2025 10:07:52 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/chaitin.hpp line 151:
> 
>> 149:     _mask_size = _mask.rm_size_in_bits();
>> 150:     return _mask.rollover();
>> 151:   }
> 
> Subjective: I would have kept the one-liner approach consistently here, since that is what surrounding code does.

Fair enough, I'll update!

> src/hotspot/share/opto/ifg.cpp line 732:
> 
>> 730: 
>> 731:     // Remove bound register(s) from 'l's choices
>> 732:     old = interfering_lrg.mask();
> 
> Just checking: This is an implicit `copy` case, right?

Right, it is equivalent to make `copy` public and call it directly. However, see my other comment for why I think we (for now) should keep the `operator=` around.

> src/hotspot/share/opto/locknode.cpp line 43:
> 
>> 41: 
>> 42: BoxLockNode::BoxLockNode(int slot)
>> 43:     : Node(Compile::current()->root()), _slot(slot),
> 
> Suggestion:
> 
>     : Node(Compile::current()->root()),
>       _slot(slot),
> 
> I would put all on separate lines. Optional.

Sure, I'll update

> src/hotspot/share/opto/locknode.cpp line 55:
> 
>> 53:   }
>> 54:   init_class_id(Class_BoxLock);
>> 55:   init_flags(Flag_rematerialize);
> 
> Any reason why you moved these after the bailout? Maybe that's fine, but I don't know what the implications might be. Do you?

No reason that I can remember. I'll move them before the bailout!

> src/hotspot/share/opto/machnode.hpp line 758:
> 
>> 756: public:
>> 757:   MachProjNode(Node* multi, uint con, const RegMask& out, uint ideal_reg)
>> 758:       : ProjNode(multi, con), _rout(out, Compile::current()->comp_arena()),
> 
> Suggestion:
> 
>       : ProjNode(multi, con),
>         _rout(out, Compile::current()->comp_arena()),
> 
> Optional. Either list horizontally or vertically is my opinion ;)

Sure, updated

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2352218757
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2352225778
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2352228859
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2352236989
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2352240162