[lworld] RFR: 8331912: [lworld] C2: assert(!had_error) failed: bad dominance with TestFlatInArraysFolding.java

[Christian Hagedorn]

On Thu, 6 Jun 2024 11:51:24 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> In this bug, we have a broken graph due to data being folded while control is not. 
> 
> #### A Problem that's (almost) Fixed 
> 
> In this particular case, a `CheckCastPP` pinned to a sub type check becomes top due not non-matching (i.e impossible) flatness information but the sub type check fails to detect that and is not being folded. This was actually fixed with [JDK-8321734](https://bugs.openjdk.org/browse/JDK-8321734) such that the `SubTypeCheckNode` is properly folded as well.
> 
> #### Still a Problem with `ExpandSubTypeCheckAtParseTime`
> This bug manifests when running the following test case of JDK-8321734 with `ExpandSubTypeCheckAtParseTime`:
> 
> https://github.com/openjdk/valhalla/blob/ec59503cc04b0a4b15f21a8d8727c6c92924d052/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestFlatInArraysFolding.java#L99-L104
> 
> We load an element from `oArrArr` which could be an inline type and thus be flat in array - but we do not know that. Therefore, we have a `Phi` that merges a flat-in-array version and a normal version.
> 
> #### How Does `ExpandSubTypeCheckAtParseTime` Work
> Setting `ExpandSubTypeCheckAtParseTime` will eagerly expand any sub type check at parse time. This means that we do not emit any `SubTypeCheckNode` but instead directly expand it. We only check once if the sub type check could be statically determined to be true or false and thus be removed:
> 
> https://github.com/openjdk/valhalla/blob/ec59503cc04b0a4b15f21a8d8727c6c92924d052/src/hotspot/share/opto/graphKit.cpp#L2858-L2879
> 
> If that's not the case - as in `testSubTypeCheck()` - we emit all the expanded nodes like checking the secondary super cache etc.
> 
> #### The Actual Problem With Eager Sub Type Check Expansion
> 
> Later, when unswitching the loop, we have one version of the loop where the `CheckCastPP` for `oArrArr[i]` is flat-in-array which goes into `373 CheckCastPP` with an array type:
> 
> ![image](https://github.com/openjdk/valhalla/assets/17833009/23109401-8f4c-46b4-a13a-c9cbc83954b6)
> 
> Arrays can never be flat in array, so the `373 CheckCastPP` is replaced by top. The corresponding `SubTypeCheckNode` would also detect this - but there is none since we already expanded it. This improved flat-in-array type information is not propagated to `365 CmpP` through the emitted sub type check nodes (e.g. secondary super cache checks etc.) after the expansion. As a result, we fail to fold the check.
> 
> #### Solution
> There is no good solution to fix this but to just not run with `ExpandSubTypeCheckAtParseTime`. We've rem...

Thanks Tobias for your review!

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

PR Comment: https://git.openjdk.org/valhalla/pull/1116#issuecomment-2152568030