[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:

![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 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