On Wed, 18 Mar 2026 17:03:27 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 speculative type as well as it
> is redundant. But this would obfuscate the fact the initial type of the first
> `CheckCastPP` is alread...
This pull request has now been integrated.
Changeset: dc15f9b0
Author: Benoît Maillard <[email protected]>
Committer: Tobias Hartmann <[email protected]>
URL:
https://git.openjdk.org/valhalla/commit/dc15f9b079e468b1d6c12cb3754a11c2db59323b
Stats: 112 lines in 4 files changed: 93 ins; 4 del; 15 mod
8367624: [lworld] C2: Missed Value() optimization for CheckCastPP
Reviewed-by: thartmann
-------------
PR: https://git.openjdk.org/valhalla/pull/2242
