RFR: 8342156: C2: Compilation failure with fewer arguments after JDK-8329032 [v2]

[Daniel Lundén]

On Tue, 22 Oct 2024 08:51:31 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:
>> 
>>   Apply suggestions from code review
>>   
>>   Co-authored-by: Roberto Castañeda Lozano <robcasloz at users.noreply.github.com>
>
> src/hotspot/share/adlc/formsopt.cpp line 177:
> 
>> 175:   // Add one more word to avoid problematic rounding. Specifically, APX added
>> 176:   // 16 more registers but did not result in a mask size increase.
>> 177:   // Round up to the next doubleword size.
> 
> I agree with Dean that the comment should state more clearly what is problematic with the current rounding. From your analysis in the JBS issue, I take it that the problem is that
> 1) the number of bits left for stack locations depends on the value of `words_for_regs`,
> 2) before the addition of APX registers we were relying on the slack created by the up-rounding to accommodate enough stack locations, and
> 3) after adding APX registers this slack has been reduced significantly.
> 
> Is this the case? If so, I suggest to update the comment to something like:
> 
> Suggestion:
> 
>   // Round up to the next doubleword size.
>   // Add one more word to accommodate a reasonable number of stack locations
>   // in the register mask regardless of how much slack is created by rounding up.

I'm fine with this change, although I'd then argue that we could simplify it and just say we add 4 words (instead of 3) for incoming/outgoing arguments. @vnkozlov You suggested the current wording that specifically mentions APX, what do you think?

> test/hotspot/jtreg/compiler/arguments/TestManyParameters.java line 43:
> 
>> 41: 
>> 42:     public static void main(String[] args) {
>> 43:         test(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54);
> 
> Is this the maximum number of arguments that C2 could handle before the addition of APX?

Yes, that's my understanding (more details are in the JBS issue). @chhagedorn: Can you confirm?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21612#discussion_r1810388578
PR Review Comment: https://git.openjdk.org/jdk/pull/21612#discussion_r1810391955