> 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...
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.
-------------
Changes:
- all: https://git.openjdk.org/valhalla/pull/2242/files
- new: https://git.openjdk.org/valhalla/pull/2242/files/642e67d6..6e248e40
Webrevs:
- full: https://webrevs.openjdk.org/?repo=valhalla&pr=2242&range=02
- incr: https://webrevs.openjdk.org/?repo=valhalla&pr=2242&range=01-02
Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod
Patch: https://git.openjdk.org/valhalla/pull/2242.diff
Fetch: git fetch https://git.openjdk.org/valhalla.git pull/2242/head:pull/2242
PR: https://git.openjdk.org/valhalla/pull/2242
