On Tue, 30 Jun 2026 13:46:33 GMT, Tobias Hartmann <[email protected]> wrote:
> When slightly modifying the test that I added for > [JDK-8386067](https://bugs.openjdk.org/browse/JDK-8386067), I hit an assert > in `CallStaticJavaNode::replace_is_substitutable` because expansion of the > substitutability call failed and `emit_substitutability_check` returned > `nullptr` although `can_emit_substitutability_check` returned `true`. > > In the new test, the problematic field is `ObjectHolder::obj` which, although > it is of type `Object`, is always `exact` and therefore can't be a value > object and does not require a substitutability test. > `InlineTypeNode::can_emit_substitutability_check` correctly returns `true`: > https://github.com/openjdk/valhalla/blob/2ae8e47da016bb4d7718e7923c964c2b91c638e6/src/hotspot/share/opto/inlinetypenode.cpp#L611-L614 > > > However, during recursive expansion, nullable value object fields are > null-checked and then compared field-by-field: > https://github.com/openjdk/valhalla/blob/2ae8e47da016bb4d7718e7923c964c2b91c638e6/src/hotspot/share/opto/inlinetypenode.cpp#L910 > > The code unconditionally rebuilt those fields with > `InlineTypeNode::make_from_oop` after the null and type checks because > there's a `CastPP` in-between: > https://github.com/openjdk/valhalla/blob/2ae8e47da016bb4d7718e7923c964c2b91c638e6/src/hotspot/share/opto/inlinetypenode.cpp#L955-L960 > > If the field was already scalarized, this discarded the existing > `InlineTypeNode` and reloaded fields from the oop, losing precise information > such as exact Object field Phis. The later Object field comparison then looks > non-exact and `emit_substitutability_check_pointer` returns `nullptr`: > https://github.com/openjdk/valhalla/blob/2ae8e47da016bb4d7718e7923c964c2b91c638e6/src/hotspot/share/opto/inlinetypenode.cpp#L722-L724 > > The fix is to preserve already-scalarized recursive field operands when their > inline klass matches the comparison klass, and only checkcast/reload operands > that are not scalarized. With the fix, exact type information is preserved > and we emit a pointer comparison for the exact object fields: > https://github.com/openjdk/valhalla/blob/2ae8e47da016bb4d7718e7923c964c2b91c638e6/src/hotspot/share/opto/inlinetypenode.cpp#L700-L702 > > The IR Framework test verifies this successful expansion. > > Note: The null and type checks are not immediately folded because by GVN > because we need to set `set_delay_transform` here: > https://github.com/openjdk/valhalla/blob/2ae8e47da016bb4d7718e7923c964c2b91c638e6/src/hotspot/share/opto/callnode.cpp#L1412-L1416 > > Thanks, > Tobias > > --------- > - [x] I c... src/hotspot/share/opto/inlinetypenode.cpp line 956: > 954: > 955: // Both must be vk > 956: if (cur_lhs_inline != nullptr && cur_lhs_inline->inline_klass() == > vk) { You can add a comment that it is not an issue if `cur_lhs_inline` is nullable and `cur_lhs_field` is null-checked, it is null-checked so that we can load the fields, and an `InlineTypeNode` does that already. test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorld.java line 5484: > 5482: > 5483: // TODO 8376254: C1 bails out if the type of the nullable flat > field is uninitialized > 5484: static ObjectHolderHolder tmp = new ObjectHolderHolder(); It would probably be better to name this more specific to its type name so as not to have name collision. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2602#discussion_r3499638809 PR Review Comment: https://git.openjdk.org/valhalla/pull/2602#discussion_r3499644577
