[lworld] RFR: 8321734: [lworld] C2: flat_in_array type is not handled correctly leading to crashes

[Tobias Hartmann]

On Fri, 19 Apr 2024 11:14:34 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> There are multiple issues around types being `flat_in_array`:
>> 
>> #### 1. `SubTypeCheckNode` is not folded while `CheckCastPP` is correctly replaced by top due to an impossible type:
>> Test: `testSubTypeCheckNotFoldedParsingAbstractClass():` 
>> 
>> We have the following:
>> - `PUnique` is the unique concrete value sub class of the abstract value `AUnique` class
>> - `Object[] arr` -> elements could be an inline type that is flat
>> 
>> We apply loop unswitching such that we have a loop where the access `arr[i]` is with a flat inline type (i.e. marked as flat-in-array) and one version without. In the flat-in-array loop, we have the following sub type check for the `checkcast` for `(AUnique)arr[i]`:
>> 
>> AUnique [maybe-flat-in-array] <: Object [flat-in-array]
>> 
>> Since we do not know if `AUnique` is flat-in-array, we cannot fold this check. However, the corresponding `CheckCastPP` for this sub type check uses the unique concrete sub class `PUnique` which is known to be *not*-flat-in-array with `FlatArrayElementMaxSize=4`. Since a not-flat-in-array type cannot be a sub type of a flat-in-array type (determined during class loading), the `CheckCastPP` is replaced by top. But the `SubTypeCheck` is not folded. 
>> 
>> This was found in Valhalla and fixed in mainline with JDK-8328480. I've added the Valhalla specific test and merged the fix from mainline into this patch.
>> 
>> #### 2. Wrong `not_flat_in_array` default value
>> Test: `testSubTypeCheck()` and `testCmpP()`:
>> Arrays are always normal objects (i.e. not inline types). They are therefore never flat-in-array and always not-flat-in-array. While they are correctly never marked as flat-in-array, they are currently not marked as *not*-flat-in-array and thus have the implicit state *maybe*-flat-in-array:
>> https://github.com/openjdk/valhalla/blob/e01ec832189453cc302c7ca8915e69bb63a3d4b1/src/hotspot/share/opto/type.hpp#L1096
>> 
>> This becomes a problem for `SubTypeCheck` and `CmpP` nodes which cannot determine that a flat-in-array klass and an array klass are unrelated. But the type system itself correctly treat arrays as *not*-flat-in-array when doing a meet operation. As a result, data is folded while control is not.
>> 
>> This is fixed by changing the flat-in-array tri-state: As a default, a type is always not-flat-in-array. Inline types then override this property accordingly.
>> 
>> #### 3. Type system wrongly treats "maybe-be-flat-in-array <: flat-in-array" as top
>> Test: `testUnswitchingAbstractClass()`
>>  "may...
>
> src/hotspot/share/opto/type.cpp line 997:
> 
>> 995: 
>> 996:   // Verify that:
>> 997:   //      this meet t == t meet this
> 
> Found these comments useful while verifying the type system changes.

Nice!

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1079#discussion_r1577738905