> 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...
Tobias Hartmann has updated the pull request incrementally with one additional commit since the last revision: New attempt at comment ------------- Changes: - all: https://git.openjdk.org/valhalla/pull/2602/files - new: https://git.openjdk.org/valhalla/pull/2602/files/1016b748..65d1515f Webrevs: - full: https://webrevs.openjdk.org/?repo=valhalla&pr=2602&range=02 - incr: https://webrevs.openjdk.org/?repo=valhalla&pr=2602&range=01-02 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.org/valhalla/pull/2602.diff Fetch: git fetch https://git.openjdk.org/valhalla.git pull/2602/head:pull/2602 PR: https://git.openjdk.org/valhalla/pull/2602
