[lworld] RFR: 8315003: [lworld] C2: Implement full support for JDK-8287061 and JDK-8316991 [v2]
Christian Hagedorn
chagedorn at openjdk.org
Thu Oct 10 08:06:40 UTC 2024
> This patch makes the improved allocation merge at EA work with inline types.
>
> Before this patch, the assumption was that we do **not** have non-scalarized inline types at safepoints which EA is going to scalarize (and if we do, then there is a missed opportunity which can be considered a bug). We should have pushed all `InlineTypeNodes` down through phis in order to have them scalarized before EA.
>
> This, however, changes with the improved allocation merge RFEs that have been merged into Valhalla ([JDK-8287061](https://bugs.openjdk.org/browse/JDK-8287061) and [JDK-8316991](https://bugs.openjdk.org/browse/JDK-8316991)). We could now the following graph as found in the test case:
>
> 
>
> Here, `272 Phi` merges the non-value type allocation `238 CheckCastPP` with the value type allocation `217 InlineType`. Before the improved allocation merge RFEs, we would not be able to remove the allocations. But now, we can do it at EA: We create a special `SafePointScalarMergeNode` which in case has `SafePointScalarObjectNodes` as inputs for the merged allocations. Additionally, each field of the merged allocations are added to the safepoint, which also includes the value type field `v2` of `V`:
>
> 
>
> We should not have safepoint uses for `InlineType` and thus now replace them with `SafePointScalarObjectNodes` as well (newly done with this patch). As a result, we have the following graph after EA:
>
> 
>
> Note that the safepoint node inputs for fields are well-defined to keep track of which field belong to each of the `SafePointScalarObjectNodes` of the `SafePointScalarMergeNode`.
>
>
> ### Additional Code Changes
> - There was a bug in mainline with the new allocation merge implementation that is already fixed in mainline but not integrated in Valhalla, yet: [JDK-8335220](https://bugs.openjdk.org/browse/JDK-8335220). I cherry-picked these changes (L589-605 in escape.cpp).
> - I added assertion code to verify that when we are going to scalar replace an inline type allocation that is an input into a phi at EA, we would not have been able to push the `InlineTypeNode` down (which should have been done earlier if possible).
> - Re-enabled uncommented tests and removed problem-listed `AllocationMergesTests.java` that had been added with ...
Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
Update test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestAllocationMergeAndFolding.java
Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
-------------
Changes:
- all: https://git.openjdk.org/valhalla/pull/1271/files
- new: https://git.openjdk.org/valhalla/pull/1271/files/b21dcca4..781e4e4d
Webrevs:
- full: https://webrevs.openjdk.org/?repo=valhalla&pr=1271&range=01
- incr: https://webrevs.openjdk.org/?repo=valhalla&pr=1271&range=00-01
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/valhalla/pull/1271.diff
Fetch: git fetch https://git.openjdk.org/valhalla.git pull/1271/head:pull/1271
PR: https://git.openjdk.org/valhalla/pull/1271
More information about the valhalla-dev
mailing list