[lworld] RFR: 8348961: [lworld] C2: Joining "Object:flat_in_array" with "Object:exact" does not properly work

Christian Hagedorn chagedorn at openjdk.org
Fri Feb 14 11:30:58 UTC 2025


This patch fixes the wrong joining operation of two types of the same klass with regard to flat in array. We have the following shape in the test case (how we end up with such a graph can be found as comments in the added tests):
 
![image](https://github.com/user-attachments/assets/3ea680fa-d93c-4994-92b7-dd1c271c0030)

The type of `341 CheckCastPP` is `Object:exact`, so we definitely know that the type is `Object` and nothing else and thus also not flat in array (Object is not a value class). For `339 CheckCastPP`, we know that it is some kind of value class but definitely not `Object` itself - it's just chosen because we do not know more about the klass.

Joining these two should result in an empty set since they are unrelated. However, the type system is not handling it correctly and we conclude that one is a sub type of the other and we later fail with an assert.

### Reworking flat in array Meet Operation
I revisited the way we meet two instruction types with regard to the flat in array property. The code was a little hard to understand, so I added a summary how the flat in array property should be treated when doing a meet or join:

https://github.com/openjdk/valhalla/blob/b6b30301998e09e61b9c05282878f0596bc2f61b/src/hotspot/share/opto/type.cpp#L4604-L4630

I added a new case for joining the same klasses (see `(FiA-J-Same)`) which was missing before and resulting in an assertion failure.

### Adding dual of `_flat_in_array`?
I kept asking myself if we should introduce a proper lattice with a dual for flat in array:

https://github.com/openjdk/valhalla/blob/b6b30301998e09e61b9c05282878f0596bc2f61b/src/hotspot/share/opto/type.cpp#L4593-L4597

While it's probably the proper way to address this long term, I think we should investigate this separately as we would need to touch quite some code. I therefore left the idea as a comment and went with a point fix on top - keeping the way we currently handle `_flat_in_array` in meet operations without a proper dual.

### Including Changes
- More detailed summary how the meet/join of `flat_in_array` is done .
- Refactored the code in `meet_instptr()` to make it more readable/clear how the `flat_in_array` property is treated.
- For better debugging:
  - Dumping `(flat in array)` also for `TypeInstKlassPtr` and not only for `TypeInstPtr`.
  - Adapting "Show types" IGV filter to also show the flat in array property (see image of graph above).

### Additionally Required Fixes
- (i) With the type system fix above, we sanely remove the conflicting data nodes but we fail to remove the corresponding control path as well. The reason is that `TypeInstKlassPtr::not_flat_in_array()` does not take exactness information into account:
https://github.com/openjdk/valhalla/blob/991ec927a2a111e5021b12be3413dfc96efbc74a/src/hotspot/share/opto/type.hpp#L1787

   `_klass->can_be_inline_klass()` will treat the klass as inexact (note that `TypeInstPtr::not_flat_in_array()` takes exactness into account). As a result, `TypeInstKlassPtr::not_flat_in_array()` called on `Object:exact` returns false even though it is statically known that this should be true (i.e. not flat in array). This is fixed by passing `klass_is_exact()` to `_klass->can_be_inline_klass()`:
https://github.com/openjdk/valhalla/blob/b6b30301998e09e61b9c05282878f0596bc2f61b/src/hotspot/share/opto/type.hpp#L1793-L1795

- (ii) Applying (i) resulted in some assertion failures because we now wrongly treat a sub type check of the form `X <: Object` as false even though this should be true for any class `X`. We check in `SubTypeCheckNode::Value()` -> `SybTypeCheckNode::sub()` the following:
https://github.com/openjdk/valhalla/blob/991ec927a2a111e5021b12be3413dfc96efbc74a/src/hotspot/share/opto/subtypenode.cpp#L53-L54

  In a failing case, `subk` is a value class that is flat in array while `superk` is `Object:exact`. Due to now passing the exactness information in `TypeInstKlassPtr::not_flat_in_array()` with (i), this returns true. We wrongly assume the classes are unrelated  due to conflicting `flat_in_array` information and thus the check is wrongly treated as false. 

  The fix I propose for this is to introduce a special `not_flat_in_array_inexact()` that reflects the old behavior of `not_flat_in_array()` to ignore exactness information.
 
I added some comments in the PR to help understanding the change better.

Thanks,
Christian

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

Commit messages:
 - update comment
 - 8348961: [lworld] C2: Joining "Object:flat_in_array" with "Object:exact" does not properly work

Changes: https://git.openjdk.org/valhalla/pull/1367/files
  Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=1367&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8348961
  Stats: 197 lines in 5 files changed: 157 ins; 2 del; 38 mod
  Patch: https://git.openjdk.org/valhalla/pull/1367.diff
  Fetch: git fetch https://git.openjdk.org/valhalla.git pull/1367/head:pull/1367

PR: https://git.openjdk.org/valhalla/pull/1367


More information about the valhalla-dev mailing list