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

[Christian Hagedorn]

On Fri, 14 Feb 2025 11:18:39 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> 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...

Thanks Tobias!

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

PR Comment: https://git.openjdk.org/valhalla/pull/1367#issuecomment-2665199641