On Tue, 30 Jun 2026 07:46:13 GMT, Tobias Hartmann <[email protected]> wrote:
>> The `PushInlineTypeDown` implementation from >> [JDK-8302217](https://bugs.openjdk.org/browse/JDK-8302217) rewires `PhiNode` >> uses to a new `InlineTypeNode` but only adds direct users to the IGVN >> worklist. Nodes whose transformations depend on deeper input type changes >> might not be re-enqueued. >> >> In the failing case, the offending shape looks like this: >> ```CmpL(OrL(CastP2X(phi_or_inline_type), CastP2X(other)), 0L)``` >> >> After the Phi input is replaced by a scalarized inline type or a not-null >> value, >> [CmpLNode::Ideal](https://github.com/openjdk/valhalla/blob/a0dc49c20ee6d20e925a107b0d3dabc7e0ba8863/src/hotspot/share/opto/subnode.cpp#L899-L922) >> can fold the corresponding `CastP2X` to the null marker or `1L`. Without >> notifying users through the normal IGVN replacement path, >> `VerifyIterativeGVN` will report a missed `CmpL` idealization. >> >> The fix makes `replace_in_uses` notify users before rewiring, and uses it >> from `PushInlineTypeDown`. >> >> Thanks, >> Tobias >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Tobias Hartmann has updated the pull request incrementally with one > additional commit since the last revision: > > Handle recursion Otherwise, the fix looks good to me, thanks! src/hotspot/share/opto/phaseX.cpp line 2491: > 2489: void PhaseIterGVN::replace_in_uses(Node* n, Node* m) { > 2490: assert(n != nullptr, "sanity"); > 2491: add_users_to_worklist(n); This is probably also beneficial for existing uses of `replace_in_uses()`. test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestPushThroughPhiNotification.java line 31: > 29: * @bug 8386067 > 30: * @enablePreview > 31: * @run driver ${test.main.class} Should be `main` to allow to run with passed in flags: Suggestion: * @run main ${test.main.class} test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestPushThroughPhiNotification.java line 58: > 56: > 57: public static void main(String[] args) { > 58: for (int i = 0; i < 50_000; i++) { Do you really need that many iterations? ------------- PR Review: https://git.openjdk.org/valhalla/pull/2593#pullrequestreview-4598593189 PR Review Comment: https://git.openjdk.org/valhalla/pull/2593#discussion_r3497246943 PR Review Comment: https://git.openjdk.org/valhalla/pull/2593#discussion_r3497228793 PR Review Comment: https://git.openjdk.org/valhalla/pull/2593#discussion_r3497231439
