On Wed, 1 Apr 2026 12:43:46 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:
>
> Update bug number in TODO
@benoitmaillard
Your change (at version 4550268fb2e825241a75d66733ddd1a86c9ec7fb) is now ready
to be sponsored by a Committer.
-------------
PR Comment: https://git.openjdk.org/valhalla/pull/2242#issuecomment-4169936835
