JDK-8340453 : C2: Improve encoding of LoadNKlass for compact headers
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 24
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2024-09-19
  • Updated: 2025-02-21
  • Resolved: 2024-11-15
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
24 b25Fixed
Related Reports
Blocks :  
Relates :  
Description
JDK-8305895 proposes using oopDesc::klass_offset_in_bytes() "as a placeholder/identifier of the special memory slice that only LoadNKlass uses" [1] and recovering and adjusting the original OOP from the input memory address to the LoadNKlass node at code emission time.

This use of klass_offset_in_bytes() is problematic, because no value of it can reflect accurately the position of the klass pointer within the mark word when using compact headers. This solution is fragile, unintuitive, and hard to maintain and reason about.

While faking the value of klass_offset_in_bytes() may be acceptable in the context of JEP 450 (an experimental feature) as a way of getting klass loading working in C2 with relatively low effort; a more accurate, robust, and maintainable model independent of klass_offset_in_bytes() should be developed for the non-experimental version of compact headers [2].

A possible alternative is to hide the object header klass pointer extraction and make it part of the LoadNKlass node semantics, as illustrated in the attached example. LoadNKlass nodes can then be expanded into more primitive operations (load and shift for compact headers, load with klass_offset_in_bytes() for original headers) within C2's back-end or even during code emission as sketched here [3].

[1] https://github.com/openjdk/jdk/pull/20677#issue-2480813371
[2] https://github.com/openjdk/jdk/pull/20677#issuecomment-2360756796
[3] https://github.com/robcasloz/jdk/commit/6cb4219f101e3be982264071c2cb1d0af1c6d754
Comments
Changeset: ff12ff53 Branch: master Author: Roman Kennke <rkennke@openjdk.org> Date: 2024-11-15 18:10:30 +0000 URL: https://git.openjdk.org/jdk/commit/ff12ff534abb2e08d1bb44a83ef4f84b8476f94c
15-11-2024

A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk/pull/22078 Date: 2024-11-13 15:08:03 +0000
13-11-2024

Oh and there seem to be cases where we do want to load from an offset that is not oopDesc::klass_offset_in_bytes(), e.g. in Phase::gen_subtype_check(). This would only ever happen with LoadKlassNode, but it would be odd to have different shapes for different variants of LoadKlass, IMO.
12-11-2024

There may be a problem with the approach. I see that the Klass* or narrowKlass is stored using StorePNode or StoreNKlassNode at klass_offset_in_bytes(). However, with this change, there would be no matching LoadKlassNode or LoadNKlassNode. I fear that this breaks memory optimizations that replace a load with the value of a correponding store. There's also a bunch of places where I find it hard to convince myself that reshaping Load(N)Klass would not subtly break optimizations.
12-11-2024