[lworld] RFR: 8331912: [lworld] C2: assert(!had_error) failed: bad dominance with TestFlatInArraysFolding.java [v2]
Christian Hagedorn
chagedorn at openjdk.org
Thu Jun 6 13:49:41 UTC 2024
> 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...
Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
Update src/hotspot/share/opto/graphKit.cpp
Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
-------------
Changes:
- all: https://git.openjdk.org/valhalla/pull/1116/files
- new: https://git.openjdk.org/valhalla/pull/1116/files/d31daad7..73369ade
Webrevs:
- full: https://webrevs.openjdk.org/?repo=valhalla&pr=1116&range=01
- incr: https://webrevs.openjdk.org/?repo=valhalla&pr=1116&range=00-01
Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
Patch: https://git.openjdk.org/valhalla/pull/1116.diff
Fetch: git fetch https://git.openjdk.org/valhalla.git pull/1116/head:pull/1116
PR: https://git.openjdk.org/valhalla/pull/1116
More information about the valhalla-dev
mailing list