On Tue, 30 Sep 2025 08:03:27 GMT, Marc Chevalier <[email protected]> wrote:
>> src/hotspot/share/opto/inlinetypenode.cpp line 171:
>>
>>> 169:
>>> 170: // Merges 'this' with 'top' by updating the input PhiNodes added by
>>> 'clone_with_phis'
>>> 171: InlineTypeNode* InlineTypeNode::merge_with_top(PhaseGVN* gvn, int
>>> pnum, bool transform) {
>>
>> I don't like the code duplication with `InlineTypeNode::merge_with` here.
>> Would it be feasible to merge the two and have `InlineTypeNode::merge_with`
>> handle `top` for `other`? Maybe the `PhiNode` inputs and `val2`can be
>> initialized to `top` by default (maybe already in `clone_with_phis`) such
>> that we only need to guard the `set_req` calls by `!other->is_top()`?
>
> If we initialize the inputs of the `PhiNodes` above `this` to top, we don't
> even need to merge with anything. We can just skip the call to `merge_with`,
> no? It seems mostly harmless to change `clone_with_phis` this way, except for
> the callsite in `Parse::ensure_phi` where I'm not sure of the consequences.
Right, skipping the call would be even better. I think the `ensure_phi`
callsite should be fine as well as all the branches that the phi inputs
correspond to should either be initialized afterwards or are dead (in which
case `top` seems appropriate). But only testing will tell :D
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1640#discussion_r2390314232
