8359378: aarch64: crash when using -XX:+UseFPUForSpilling

[bulasevich]

AArch64 BarrierSetAssembler path assumes only FP/vector ideal regs reach the FP spill/restore encoding. With -XX:+UseFPUForSpilling Register Allocator may allocate scalar values in FP registers. When such values (Op_RegI/Op_RegN/Op_RegL/Op_RegP) hit BarrierSetAssembler::encode_float_vector_register_size, we trip ShouldNotReachHere in release build and "unexpected ideal register" assertion in debug build.

Fix: teach the encoder to handle scalar ideal regs when they physically live in FP regs:

• treat Op_RegI / Op_RegN as 32-bit (single slot) - same class as Op_RegF
• treat Op_RegL / Op_RegP as 64-bit (two slots) - same class as Op_RegD

Related:

• reproduced since 8334060: Implementation of Late Barrier Expansion for G1 #19746
• spilling logic:
  • 8330685: ZGC: share barrier spilling logic #18967
  • 8326541: [AArch64] ZGC C2 load barrier stub should consider the length of live registers when spilling registers #17977

Testing: tier1-3 with javaoptions -Xcomp -Xbatch -XX:+UseFPUForSpilling on AARCH

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8359378: aarch64: crash when using -XX:+UseFPUForSpilling (Bug - P3)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27350/head:pull/27350
$ git checkout pull/27350

Update a local copy of the PR:
$ git checkout pull/27350
$ git pull https://git.openjdk.org/jdk.git pull/27350/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27350

View PR using the GUI difftool:
$ git pr show -t 27350

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27350.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back bulasevich! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@bulasevich The following label will be automatically applied to this pull request:

• hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (9503349d)

[robcasloz]

Hi @bulasevich, thanks for working on this issue, but please note that it was already assigned to me (JDK-8359378). I am fine with re-assigning it to you, but next time please ask first, to avoid work duplication.

[robcasloz]

/cc hotspot-compiler

[openjdk]

@robcasloz
The hotspot-compiler label was successfully added.

[bulasevich]

Right, @robcasloz,
I started investigating this issue thinking it was something wrong in my own code. Once I realized it was a common issue already assigned, I decided to propose a fix since it looked a bit abandoned. I didn’t mean to bypass your work -- you’re right, I should have contacted you first.
Anyway, I’d appreciate your review. Do you think my change is reasonable? If not, let me close this PR and leave it to you.

[theRealAph]

Given that you're looking at this, I'd appreciate it if you could form an opinion bout whether this option is of any use.

UseFPUForSpilling on AArch64 is showing signs of code rot. If it has advantages on some machine we should turn it on by default; if it does not, why support it at all?

[bulasevich]

@theRealAph Andrew, I agree with you. From my experience it is useless on Cortex-A72, Neoverse N1, Neoverse V1. I have now also checked on Neoverse V2 and Apple M4 - in both cases UseFPUForSpilling shows a clear performance degradation.

[robcasloz]

Right, @robcasloz, I started investigating this issue thinking it was something wrong in my own code. Once I realized it was a common issue already assigned, I decided to propose a fix since it looked a bit abandoned. I didn’t mean to bypass your work -- you’re right, I should have contacted you first. Anyway, I’d appreciate your review. Do you think my change is reasonable? If not, let me close this PR and leave it to you.

Thanks, I had planned to look at this in the upcoming weeks but did not start yet. I just reassigned the issue to you, will have a look at your fix within the next days.

[adinn]

I was wondering about that. So, perhaps a better fix is to change the command line ergonomics so that AArch64 either 1) refuses to run with it set to true or 2) prints a warning and resets it to false.

[bulasevich]

I suggest handling this in two steps:

• In JDK 25 we fix the crash when UseFPUForSpilling is enabled.
• In the next release we prohibit the option softly: if it is set on the command line, the VM prints a warning and resets it to false. Proposed change for the latter:

diff --git a/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp b/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp
index 308deeaf5e2..7702988c11c 100644
--- a/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp
@@ -621,4 +621,9 @@ void VM_Version::initialize() {
     FLAG_SET_DEFAULT(UseVectorizedHashCodeIntrinsic, true);
   }
+
+  if (UseFPUForSpilling) {
+    warning("UseFPUForSpilling is known to degrade performance on this platform and will be ignored.");
+    FLAG_SET_DEFAULT(UseFPUForSpilling, false);
+  }
 #endif