[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