[lworld] RFR: 8339302: [lworld] C2: "assert(safepoints.length() == 0 || !res_type->is_inlinetypeptr()) failed: Inline type allocations should not have safepoint uses" with circular inline types

Christian Hagedorn chagedorn at openjdk.org
Tue Sep 3 09:28:08 UTC 2024


The circular inline type test cases show that two asserts are too strong: They assume that we can always replace inline types with `SafePointScalarObject` nodes before doing EA with its scalar replacement phase. If we fail to do so, then there must be a missed optimization opportunity. However, this only holds for non-circular inline types. When having circular inline types, we stop the (eager) scalarization at depth 1 because we could possibly do it endlessly in a recursion. Therefore, we could have a non-scalarized `CheckCastPP` inline type oop in a safepoint which triggers the asserts.

I propose to relax the assert by excluding circular inline types.

#### Additional Required Fix

I also had to adjust the code which creates a `SafePointScalarObject` in the scalar replacement after EA for such a circular inline type represented by a non-`InlineTypeNode` as follows.

Normally, we would call `InlineTypeNode::make_scalar_in_safepoint()` to scalarize an inline type. This adds an additional input to the safepoint for the `IsInit` input to check:

https://github.com/openjdk/valhalla/blob/884078f06a2bf15e4be768dd09d0790b63d8e015/src/hotspot/share/opto/inlinetypenode.cpp#L280-L287

For example, in `testCircularSafepointUse()`, `IsInit` is false for the inline type created for `v` at L3318 and we add a top input when creating `260 SafePointScalarObject`:

https://github.com/openjdk/valhalla/blob/d719d86c33f87bbdb37c5a4055fbbb5f81d7937f/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestNullableInlineTypes.java#L3315-L3321

![image](https://github.com/user-attachments/assets/6defb530-9a37-4701-a704-591ab391a3da)

The first field `v` is the `166 CheckCastPP` oop and the second field `i` has the value `188 ConI #23`. 

But when scalarizing the inline type field `v` later (i.e. `166 CheckCastPP`) after EA, without an `InlineTypeNode`, we miss to account for `IsInit`. We create a new `264 SafePointScalarObject` and directly append the fields after the last input (i.e. `188 ConI`) without `IsInit`:

![image](https://github.com/user-attachments/assets/d5c26975-5847-4bb3-b53e-102af1471acd)

The first field `v` is a `21 ConP #null` and the second field `i` is the value `74 ConI #34`.

Later, when processing the `264 SafePointScalarObject` at code generation, we try to read the `IsInit` input which was never added:

https://github.com/openjdk/valhalla/blob/884078f06a2bf15e4be768dd09d0790b63d8e015/src/hotspot/share/opto/output.cpp#L809-L816

The first input is `21 ConP #null` which is neither `top` nor an `int` and we crash at L816 when trying to convert the type to a `TypeInt`. 

The fix I propose for that is to add a fake top input for circular inline type nodes when scalarizing them after EA such that we can skip over the `IsInit` which does not exist for the inline type oop.

Thanks,
Christian

-------------

Commit messages:
 - 8339302: [lworld] C2: "assert(safepoints.length() == 0 || !res_type->is_inlinetypeptr()) failed: Inline type allocations should not have safepoint uses" with circular inline types

Changes: https://git.openjdk.org/valhalla/pull/1231/files
  Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=1231&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8339302
  Stats: 71 lines in 2 files changed: 68 ins; 1 del; 2 mod
  Patch: https://git.openjdk.org/valhalla/pull/1231.diff
  Fetch: git fetch https://git.openjdk.org/valhalla.git pull/1231/head:pull/1231

PR: https://git.openjdk.org/valhalla/pull/1231


More information about the valhalla-dev mailing list