On Fri, 10 Apr 2026 07:19:42 GMT, Roland Westrelin <[email protected]> wrote:

> I propose transforming:
> 
> (InlineType .... (InlineType oop
> 
> 
> where an `InlineType` references another `InlineType` node (maybe itself) 
> through its `oop` input and with `Phi` or cast nodes in the subgraph in 
> between into:
> 
> 
> (InlineType .... oop
> 
> which requires cloning every cast or `Phi` nodes in the subgraph between the 
> 2 `InlineType` nodes. That prevent sinfinite invocation of 
> `PhiNode::push_inline_types_through` and cleans up the graph.
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

It seems fine to me. I'm rather convinced by the explanation, and I think this 
PR implements that. But it's really hard for me to see whether something is 
missing, some case is not handled...

I'll let Tobias take a look.

I just have a tiny style suggestion you can ignore if you don't like.

src/hotspot/share/opto/cfgnode.cpp line 2895:

> 2893: // (InlineType (Cast (Phi (InlineType oop
> 2894: // and replaces it with:
> 2895: // (InlineType (Cast (Phi oop

https://xkcd.com/859/

(just kidding)))))))))

src/hotspot/share/opto/cfgnode.cpp line 2949:

> 2947:           continue;
> 2948:         }
> 2949:         _nodes_from_phi.push(in);

What would you think about a form like

if (Node* in = n->in(1); in != nullptr) {
  _nodes_from_phi.push(in);
}

On top of being a bit shorter, it rather insist on what we do than the 
`continue` (that is not the interesting case). Feel free to ignore. No strong 
opinion.

-------------

PR Review: 
https://git.openjdk.org/valhalla/pull/2316#pullrequestreview-4408614511
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2316#discussion_r3340313634
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2316#discussion_r3340451842

Reply via email to