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

Daniel Lundén dlunden at openjdk.org
Tue Aug 20 14:32:26 UTC 2024


On Mon, 19 Aug 2024 13:29:18 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> Daniel Lundén has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove leftover CHUNK_SIZE reference
>
> src/hotspot/share/opto/regmask.hpp line 129:
> 
>> 127:     } else {
>> 128:       assert(_RM_UP_EXT != nullptr, "sanity");
>> 129:       assert( i >= _RM_SIZE, "sanity");
> 
> This assertion is trivial (in my opinion) and can be removed.

Indeed, it's a leftover from a previous iteration. Thanks.

> src/hotspot/share/opto/regmask.hpp line 208:
> 
>> 206: 
>> 207:   // Set a range of words in the register mask to a given value.
>> 208:   void _set_range(unsigned int start, int value, unsigned int range) {
> 
> Suggestion: `length`, `words`, or similar would be a more idiomatic name for the third parameter (`range`), in my opinion.

Thanks, updated.

> src/hotspot/share/opto/regmask.hpp line 278:
> 
>> 276:     // itself as the _all_stack flag. We need to record this fact using the now
>> 277:     // separate _all_stack flag.
>> 278:     set_AllStack(_RM_UP[_RM_MAX] & (uintptr_t(1) << _WordBitMask));
> 
> As we discussed offline, I suggest simplifying this by updating the ADLC code as well to set `_all_stack` explicitly. Here is a patch that accomplishes that: https://github.com/robcasloz/jdk/commit/835628893ed5ea838c16afa49adbd57e4152a5dd, feel free to merge if you agree. The patch passes tier1-3 for all Oracle-supported platforms.

Yes, nice cleanup. Now included!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r1723421527
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r1723423140
PR Review Comment: https://git.openjdk.org/jdk/pull/20404#discussion_r1723422754


More information about the hotspot-compiler-dev mailing list