[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:
> 
> ![image](https://github.com/user-attachments/assets/4e5174d3-9a52-4c43-8dcd-b6a119438ca0)
> 
> 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`:
> 
> ![image](https://github.com/user-attachments/assets/eb8e7012-262d-492f-ae30-a3962f5ae931)
> 
> 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:
> 
> ![image](https://github.com/user-attachments/assets/0cf521c4-dee1-4cb7-bb7c-3479b459ed34)
> 
> 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