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