JDK-8342156 : C2: Compilation failure with fewer arguments after JDK-8329032
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 24
  • Priority: P3
  • Status: In Progress
  • Resolution: Unresolved
  • Submitted: 2024-10-15
  • Updated: 2024-10-22
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 24
24Unresolved
Related Reports
Relates :  
Relates :  
Relates :  
Description
# Failure analysis

The addition of the new APX registers results in less available space in register masks for method arguments. The (static) register mask size computation in RegisterForm::RegMask_Size does take the number of available registers into account, but also rounds up to an even number of 32-bit words. Specifically, the register mask size computation is

  (words_for_regs + 3 + 1) & ~1;

Adding the new APX registers correctly bumps `words_for_regs` from 18 to 19, but due to the rounding, the result is the same (22) as for a value of 18 for `words_for_regs`.

# Original description

JDK-8329032 introduced register allocation support for APX. APX adds 16 new 64 bit general purpose register. Thus, JDK-8329032 increased `Register::number_of_registers` from 16 to 32. As a result, ConcreteRegisterImpl::number_of_registers is now increased by 32. Consequently, the `VMRegImpl::FRIST_STACK` is also increased by 32 and `stack2reg()` starts at a number that is larger by 32 than before JDK-8329032. 

This means that there is less stack space left to spill arguments to the stack. In `Matcher::warp_outgoing_stk_arg()`, we check with `RegMask::can_represent_arg(warped)` if there is enough space left on the stack for all argument and bail out if not.

Before JDK-8329032, we could C2 compile a method call with 54 arguments but now we are only able to compile with 38 arguments (see attached Test.java). This seems to be a regression and should be fixed even though there is JDK-8325467 which will generally solve the problem of bailing out with too many arguments.

While looking at JDK-8329032, I noticed that there is an additional method `Register::vailable_gp_registers()` which accounts for UseAPX. But it does not seem to be used in places where we use `Register::number_of_registers` (even when there is no APX support). Should this be fixed as well?

This was found while merging the change into Valhalla which caused some unexpected compilation bailouts (JDK-8342064).
Comments
As I've discussed offline with [~kvn], we determine the register mask size at build time, so dynamically sizing based on UseAPX is currently not possible. Also, [~thartmann] thought that we should not mark this as a duplicate of JDK-8325467 to ease backporting. I think that sounds like a good idea. I've submitted a simple PR for review with my above suggestion. I'm convinced this is the best solution (until integration of JDK-8325467). Regarding the comment "While looking at JDK-8329032, I noticed that there is an additional method `Register::available_gp_registers()` which accounts for UseAPX. But it does not seem to be used in places where we use `Register::number_of_registers` (even when there is no APX support). Should this be fixed as well?" from [~chagedorn]. I'd say that, if this is a problem (any input on this [~jbhateja]?), we should file a new separate follow-up issue.
21-10-2024

A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk/pull/21612 Date: 2024-10-21 14:05:54 +0000
21-10-2024

As we discussed offline, the other solution is calculate RegMask size dynamically based on UseAPX flag value.
15-10-2024

Can we consider this as duplicate of JDK-8325467 ? Or [~dlunden] suggestion about RegMask_Size change is enough?
15-10-2024

As [~chagedorn] mentions, my fix for JDK-8325467 would fix this issue as well. Regarding the regression, it is a bit strange that we went from being able to compile 54 arguments to just being able to compile 38. We do account for all registers in the AD-files when computing the static size of register masks (see "words_for_regs" in RegisterForm::RegMask_Size). My guess is that the regression is a simple rounding issue, as we round up the static mask size (in 32-bit words) to an even number. A solution could be to bump the 3 in return (words_for_regs + 3 + 1) & ~1; at the end of RegisterForm::RegMask_Size() to 4 or 5. Based on some benchmarking I did for JDK-8325467, the performance impact should be negligible.
15-10-2024

Good catch, Christian!
15-10-2024

ILW = C2 compilation bailout with many arguments, only with many arguments, no workaround = MMH = P3
15-10-2024

[~jbhateja] can you have a look?
15-10-2024