[lworld] RFR: [lworld] C2: Support abstract value class fields

[Christian Hagedorn]

On Wed, 4 Sep 2024 10:32:49 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> This patch adds support for abstract value class fields. To make it work, it required fixes in various places. The two main things to change was to account for fields in abstract value classes when collecting/accessing all fields which was not expected before and updating the way we handle larvals.
> 
> ## Updated Larval Handling
> ### Previous Larval Initialization
> Before this change, larvals were always updated inside a single concrete value class constructor. It could have been done directly:
> 
> new MyValue -> MyValue.<init> (initialization here) -> Object.<init>
> 
> or by first delegating to another value type constructor inside the same value class which does the initiliazation:
> 
> new MyValue -> MyValue.<init> -> MyValue.<init> (initialization here) -> Object.<init>
> 
> It was not possible to distribute the initialization of an inline type to multiple constructors (i.e. at value class constructor cannot do a partial initialization).
> 
> Therefore, we could just detect any call from a value class constructor to an abstract constructor or the `Object` constructor with the same receiver and then mark the inline type as non-larval since it's completely initialized before calling the super constructor (abstract classes and `Object.<init>` cannot modify the inline type anymore). The only thing required after the call was to detect if we actually inlined another constructor with the same receiver within the same concrete value class. If that's the case, we need to update the exit map to propagate the initialized receiver to the caller.
> 
> This now changes: With fields in abstract value classes, a larval is not completely initialized after the concrete value class constructor is complete. We therefore needed to find a new way to ensure the correct initialization of larvals.
> 
> ### New Larval Initialization with Fields in Abstract Classes
> #### Problems
> - A larval _could_ be fully initialized after calling the concrete value class constructor. But we could also have a situation where we still need to initialize a field in an abstract value class. Therefore, we cannot eagerly mark an inline type as non-larval once the concrete value class constructor is completed nor when an abstract value class constructor is completed (there could be multiple).
> - The currently parsed method only has the map/JVM state from the direct caller. We therefore cannot simply detect the call to the `Object.<init>()` call at the end of chained value class constructor calls and then propagate the update to al...

src/hotspot/share/ci/ciInstanceKlass.hpp line 72:

> 70:   ciConstantPoolCache*   _field_cache;  // cached map index->field
> 71:  public:
> 72:   GrowableArray<ciField*>* _nonstatic_fields;

Could probably be improved in the future to avoid this direct public access.

src/hotspot/share/compiler/methodLiveness.cpp line 627:

> 625:           (method->is_object_constructor() &&
> 626:            ((!holder->is_abstract() && holder->is_inlinetype()) ||
> 627:             holder->is_abstract()))) {

We now keep more receivers live that might not be needed (non-value abstract classes). But I think this is the simplest/most straight forward solution since we do not have access to the actual receiver node here to check its bottom type as done in the code during parsing.

src/hotspot/share/oops/inlineKlass.cpp line 249:

> 247:   int count = 0;
> 248:   SigEntry::add_entry(sig, T_METADATA, name(), base_off);
> 249:   for (JavaFieldStream fs(this); !fs.done(); fs.next()) {

`JavaFieldStream` failed to get the fields in the abstract class. Using `HierarchicalFieldStream` fixes this.

src/hotspot/share/opto/doCall.cpp line 581:

> 579:   bool      call_does_dispatch = false;
> 580: 
> 581:   // Detect the call to the object or abstract class constructor at the end of a value constructor to know when we are done initializing the larval

Removed pre-call processing. Everything done post-call now.

src/hotspot/share/opto/graphKit.cpp line 1897:

> 1895:     } else if (arg->is_InlineType()) {
> 1896:       // Pass inline type argument via oop to callee
> 1897:       InlineTypeNode* inline_type = arg->as_InlineType();

Before this patch, we already marked an inline type as non-larval before the non-inlined super constructor call. So, we would always initialize the buffer when calling the super constructor. But now, we could still have marked larvals on which we call the super constructors. We therefore need to force an initialization in this case.

src/hotspot/share/opto/graphKit.cpp line 1973:

> 1971: 
> 1972:   // We just called the constructor on a value type receiver. Reload it from the buffer
> 1973:   if (call->method()->is_object_constructor() && call->method()->holder()->is_inlinetype()) {

`call->method()->holder()->is_inlinetype()` false for abstract value classes -> use bottom type of receiver instead.

src/hotspot/share/opto/inlinetypenode.cpp line 1348:

> 1346:     st->print(" #larval");
> 1347:   }
> 1348: }

Added for easier debugging.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1236#discussion_r1743498014
PR Review Comment: https://git.openjdk.org/valhalla/pull/1236#discussion_r1743504878
PR Review Comment: https://git.openjdk.org/valhalla/pull/1236#discussion_r1743499724
PR Review Comment: https://git.openjdk.org/valhalla/pull/1236#discussion_r1743505840
PR Review Comment: https://git.openjdk.org/valhalla/pull/1236#discussion_r1743520647
PR Review Comment: https://git.openjdk.org/valhalla/pull/1236#discussion_r1743507515
PR Review Comment: https://git.openjdk.org/valhalla/pull/1236#discussion_r1743527757