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

Emanuel Peter epeter at openjdk.org
Wed Apr 23 07:15:55 UTC 2025


On Wed, 23 Apr 2025 06:14:03 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Daniel Lundén has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Refactor and improve TestNestedSynchronize.java
>
> src/hotspot/share/opto/node.cpp line 558:
> 
>> 556:     MachProjNode* mach = n->as_MachProj();
>> 557:     MachProjNode* mthis = this->as_MachProj();
>> 558:     new (&mach->_rout) RegMask(mthis->_rout);
> 
> Hmm. Do you have some defense against these kinds of shallow copies? Or are there cases where shallow copies of a RegMask are somehow valid? You could for example `nullptr` our the pointers in the copy constructor. Just an idea, not sure if that even works. Or you could do a proper copy in the copy constructor. Maybe you have already thought about both of those options, and neither are good...

Ah, I see you have a `uintptr_t** orig_ext_adr = &_RM_UP_EXT;` trick. So shallow copies should not ever get used, right?

> src/hotspot/share/opto/regmask.cpp line 395:
> 
>> 393: 
>> 394: #ifndef PRODUCT
>> 395: bool RegMask::_dump_end_run(outputStream* st, OptoReg::Name start,
> 
> In my understanding we only use underscore for fields, and not methods?

I think just making it private is sufficient, no?

> src/hotspot/share/opto/regmask.hpp line 193:
> 
>> 191:   // In this example, registers {r5, r6} and stack locations {s0, s2, s3, s5}
>> 192:   // are included in the register mask. Depending on the value of _all_stack,
>> 193:   // (s10, s11, ...) are all included (as = 1) or excluded (as = 0). Note that
> 
> Maybe I missed it: what is `as`?

Is it an abbreviation for something?

> src/hotspot/share/opto/regmask.hpp line 199:
> 
>> 197:   //
>> 198:   // The only operation that may update the _offset attribute is
>> 199:   // RegMask::rollover(). This operation requires the register mask to be clean
> 
> What does `clean` mean? All zero? Or all ones?

I think all ones. You could write `... mask to be clean (all ones) and ...`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2055352711
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2055332735
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2055366456
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r2055371270


More information about the hotspot-compiler-dev mailing list