[lworld] RFR: 8331912: [lworld] C2: assert(!had_error) failed: bad dominance with TestFlatInArraysFolding.java
Christian Hagedorn
chagedorn at openjdk.org
Thu Jun 6 11:57:25 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:

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 removed `ExpandSubTypeCheckAtParseTime` in mainline with [JDK-8332032](https://bugs.openjdk.org/browse/JDK-8332032) due to this reason and other maintenance cost considerations in general. I'm therefore now downstreaming this patch already to fix this. It needed some adaptations due to missing code in Valhalla and Valhalla specific occurrences.
Thanks,
Christian
-------------
Commit messages:
- 8331912: [lworld] C2: assert(!had_error) failed: bad dominance with TestFlatInArraysFolding.java
Changes: https://git.openjdk.org/valhalla/pull/1116/files
Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=1116&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8331912
Stats: 16 lines in 4 files changed: 7 ins; 6 del; 3 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