[lworld] RFR: 8332406: [lworld] C2: Turn flat in array property into proper lattice in type system to address remaining issues

Mon Dec 8 08:48:22 UTC 2025

On Mon, 8 Dec 2025 08:13:34 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> This patch turns the boolean flag `_flat_in_array` into a new `FlatInArray` enum to properly define a (Boolean) lattice. This mainly allows us to cleanly represent "maybe flat in array" and "not flat in array". The dedicated top element is the dual of "maybe flat in array".
> 
> To simplify the review and to better understand what the changes are, I added code comments directly in the PR.
> 
> Here is a high-level overview of the changes:
> - `FlatInArray` enum to define a lattice for `flat_in_array`:
> https://github.com/openjdk/valhalla/blob/6b1930c7b9a359223a998d0086a1326a9d7905e7/src/hotspot/share/opto/type.hpp#L1182-L1199
> - Updated `meet` operations accordingly. Needed to be careful when doing the meet above the centerline.
> - Added single `compute_flat_in_array()` method to properly compute the `flat_in_array` property from the information we have when we need to set flat in array newly (this was no properly done before).
> - In `flatten_alias_type()`, we flatten to "maybe flat in array".
> 
> #### Testing:
> - t1-4 + valhalla-comp-stress
> 
> Thanks for your feedback,
> Christian

src/hotspot/share/ci/ciInlineKlass.cpp line 41:

> 39: 
> 40: // Are arrays containing an instance of this value class always flat?
> 41: bool ciInlineKlass::is_always_flat_in_array() const {

Used for `compute_flat_in_array()`.

src/hotspot/share/ci/ciInstanceKlass.cpp line 708:

> 706:   if (!is_exact) {
> 707:     // Not exact, check if this is a valid super for an inline klass
> 708:     GUARDED_VM_ENTRY(

Is now called from more paths over `compute_flat_in_array()`. As a result, hit assert during testing because thread was already in VM state - changed to `GUARDED_VM_ENTRY`.

src/hotspot/share/opto/subtypenode.cpp line 53:

> 51:   bool unrelated_classes = false;
> 52:   // Handle inline type arrays
> 53:   if (subk->flat_in_array() && superk->not_flat_in_array_inexact()) {

The following logic was hidden in `not_flat_in_array_inexact()` and not completely accurate. Improved now.

src/hotspot/share/opto/type.cpp line 1068:

> 1066:     // 2)
> 1067:     tty->print("mt_dual meet this_dual= "); t2this        ->dump(); tty->cr();
> 1068:     tty->cr();

Since I hit the assert a few times while working on this patch, I added some more printing code for easier debugging. I guess it could also be useful in the future.

src/hotspot/share/opto/type.cpp line 3211:

> 3209:       if (!Verbose) {
> 3210:         break;
> 3211:       }

Can be discussed which state we want to dump by default. It might be too verbose to always dump the exact state.

src/hotspot/share/opto/type.cpp line 4169:

> 4167:          "cannot have constants with non-loaded klass");
> 4168:   assert(!klass()->maybe_flat_in_array() || flat_in_array, "Should be flat in array");
> 4169:   assert(!flat_in_array || can_be_inline_type(), "Only inline types can be flat in array");

Is now checked with `compute_flat_in_array()`.

src/hotspot/share/opto/type.cpp line 4268:

> 4266:   if (flat_in_array == Uninitialized) {
> 4267:     flat_in_array = compute_flat_in_array(ik, xk);
> 4268:   }

We use the special `Uninitialized` state to mark that we should compute the flat in array property from the information we got. `Uninitialized` has no other use.

src/hotspot/share/opto/type.cpp line 4517:

> 4515: 
> 4516: template<class T> TypePtr::MeetResult TypePtr::meet_instptr(PTR& ptr, const TypeInterfaces*& interfaces, const T* this_type, const T* other_type,
> 4517:                                                             ciKlass*& res_klass, bool& res_xk, bool& res_flat_in_array) {

This method now becomes much simpler without handling the meet of flat in array.

src/hotspot/share/opto/type.cpp line 4533:

> 4531:     bool res_xk = false;
> 4532:     const Type* res;
> 4533:     MeetResult kind = meet_instptr(ptr, interfaces, this, tinst, res_klass, res_xk);

I removed the meet of flat in array from `meet_instptr()` into a separate `meet_flat_in_array()` to simplify the logic.

src/hotspot/share/opto/type.cpp line 4744:

> 4742: const Type* TypeInstPtr::xdual() const {
> 4743:   return new TypeInstPtr(dual_ptr(), klass(), _interfaces, klass_is_exact(), const_oop(), dual_offset(),
> 4744:                          dual_flat_in_array(_flat_in_array), dual_instance_id(), dual_speculative(), dual_inline_depth());

Take proper dual for `_flat_in_array`.

src/hotspot/share/opto/type.cpp line 4836:

> 4834:   }
> 4835:   return TypeOopPtr::empty();
> 4836: }

Since we have now a proper `TopFlat` element in the lattice, we can introduce an `empty()` method to check if the type is above the centerline and thus empty.

src/hotspot/share/opto/type.cpp line 4878:

> 4876: }
> 4877: 
> 4878: const TypeInstPtr *TypeInstPtr::cast_to_maybe_flat_in_array() const {

Used by `flatten_alias_type()`.

src/hotspot/share/opto/type.cpp line 5395:

> 5393:     case Constant:
> 5394:     case NotNull:
> 5395:     case BotPTR: { // Fall down to object klass

Required to add braces, otherwise, this does not build.

src/hotspot/share/opto/type.cpp line 6225:

> 6223:   if (flat_in_array() && !klass()->is_inlinetype()) {
> 6224:     st->print(" (flat in array)");
> 6225:   }

Now done in `TypeInstKlassPtr::dump2()`.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597494142
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597503433
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597511101
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597514427
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597519218
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597521681
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597525113
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597532200
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597529087
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597535200
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597539928
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597541428
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597559003
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597560905