On Thu, 26 Mar 2026 11:59:06 GMT, Benoît Maillard <[email protected]> wrote:
>> This PR prevents creating a missed optimization opportunity that originates
>> from creating inconsistent `TypeArrayPtr` types.
>>
>> ## Context
>>
>> The link between the root cause of this issue and how it gets reported as a
>> missing optimization opportunity is quite subtle.
>>
>> We initially have a chain of two `CheckCastPP` nodes, where the first one is
>> a cast to `not null free` and the second one is a cast to `flat`:
>>
>> ...
>> CheckCastPP
>> |
>> CheckCastPP
>> ...
>>
>>
>> Let's look at what happens in `ConstraintCastNode::Value`:
>>
>> In the initial GVN pass of `ConstraintCastNode::Value` for the **second
>> node**, we have (only looking at `not_null_free`):
>>
>> - `in_type`
>> - `not_null_free` is `true`
>> - `not_null_free` (speculative) is `false`
>> - `_type`
>> - `not_null_free` is `false`
>> - `not_null_free` (speculative, implicitly) is `false`
>> - `ft` (join of `in_type` and `_type`)
>> - `not_null_free` is `true`
>> - `not_null_free` (speculative) is `false`
>>
>> Note that here the speculative part of `ft` is eventually dropped because in
>> `cleanup_speculative` we keep the speculative part only if it contains
>> information about flat-/nullability:
>> ```c++
>> const Type* TypeAryPtr::cleanup_speculative() const {
>> if (speculative() == nullptr) {
>> return this;
>> }
>> // Keep speculative part if it contains information about flat-/nullability
>> const TypeAryPtr* spec_aryptr = speculative()->isa_aryptr();
>> if (spec_aryptr != nullptr && !above_centerline(spec_aryptr->ptr()) &&
>> (spec_aryptr->is_not_flat() || spec_aryptr->is_not_null_free())) {
>> return this;
>> }
>> return TypeOopPtr::cleanup_speculative();
>> }
>>
>>
>> In the IGVN verification, we have a second pass of
>> `ConstraintCastNode::Value`:
>> - `in_type` (same as first pass)
>> - `not_null_free` is `true`
>> - `not_null_free` (speculative) is `false`
>> - `_type` (changed)
>> - `not_null_free` is `true`
>> - `not_null_free` (speculative, implicitly) is `true`
>> - `ft` (join of `in_type` and `_type`)
>> - `not_null_free` is `true`
>> - `not_null_free` (speculative) is `true`
>>
>> This time the speculative type is kept, however, because it contains
>> information about not-nullability.
>>
>> Now the second pass happens in IGVN verification, and we hit the missed
>> optimization opportunity assert because we are not expected to make progress
>> again (the input has not changed in the meantime).
>>
>> ## Root cause
>>
>> Now we could arguably clean up the resulting specul...
>
> Benoît Maillard has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Reapply "8380395: [lworld] ProblemList 4 sub-tests of
> compiler/valhalla/inlinetypes/TestCallingConvention.java"
>
> This reverts commit 3a54a623d6c60209a3110fec0a88f32fe56d8d7c.
But the problem list bug id should be updated to
[JDK-8380927](https://bugs.openjdk.org/browse/JDK-8380927), right? Also, can we
somehow avoid the problem listing by adjusting the test? For example, by
setting `-XX:VerifyIterativeGVN=0`. It's unfortunate, that full
`TestCallingConvention.java` is problem listed due to this intermittent issue.
It will significantly reduce our test coverage.
-------------
PR Comment: https://git.openjdk.org/valhalla/pull/2242#issuecomment-4152419925
