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

Tobias Hartmann thartmann at openjdk.org
Mon Dec 8 09:37:33 UTC 2025


On Mon, 8 Dec 2025 08:55:23 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
>
> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   update description

Very nice! I only added a few minor comments. Great, that this code is finally cleaned up.

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

> 2844: };
> 2845: 
> 2846: const char *const TypePtr::flat_in_array_msg[Uninitialized] = {

Suggestion:

const char* const TypePtr::flat_in_array_msg[Uninitialized] = {

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

> 3152:       assert(can_be_inline_type(), "only value objects can be flat in array");
> 3153:       assert(!instance_klass->is_inlinetype() || instance_klass->as_inline_klass()->is_always_flat_in_array(),
> 3154:              "a value object is only marked flat in array if it proven to be always flat in array");

Suggestion:

             "a value object is only marked flat in array if it's proven to be always flat in array");

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

> 4569:   // centerline and or-ed above it.  (N.B. Constants are always exact.)
> 4570: 
> 4571:   // Flat in Array property _flat_in_array.

Nice, that this is gone now :slightly_smiling_face:

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

> 5402:         // below the centerline when the superclass is exact. We need
> 5403:         // to do the same here.
> 5404:         //

Suggestion:

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

> 6270: 
> 6271: uint TypeInstKlassPtr::hash(void) const {
> 6272:   return klass()->hash() + TypeKlassPtr::hash() + (uint)_flat_in_array;

You might want to use `static_cast<uint>` here, similar to `TypeInstPtr::hash`.

src/hotspot/share/opto/type.hpp line 1331:

> 1329:   virtual bool maybe_null() const { return meet_ptr(Null) == ptr(); }
> 1330: 
> 1331:   static FlatInArray dual_flat_in_array(FlatInArray flat_in_array) {

I think this should be a non-static method so that you can access the `_flat_in_array` field directly.

src/hotspot/share/opto/type.hpp line 2050:

> 2048:   virtual bool eq(const Type *t) const;
> 2049: 
> 2050: 

Suggestion:

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

Marked as reviewed by thartmann (Committer).

PR Review: https://git.openjdk.org/valhalla/pull/1774#pullrequestreview-3550862668
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597656075
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597657401
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597669879
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597788559
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597710620
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597688028
PR Review Comment: https://git.openjdk.org/valhalla/pull/1774#discussion_r2597783100


More information about the valhalla-dev mailing list