RFR: 8346184: C2: assert(has_node(i)) failed during split thru phi

[Christian Hagedorn]

On Wed, 18 Dec 2024 16:57:45 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> The assert fires during split thru phi because a call to `Identity`
> returns a new node (a constant null pointer). That happens because a
> `Load`, once pushed thru phi, can be constant folded because it loads
> from a newly allocated array. `Identity` shouldn't return new
> nodes. When split thru phi runs, in this case, `Value` should be the
> one returning constant null, not `Identity`. There is logic for that
> in `LoadNode::Value` but it's after some other checks that cause
> `Value` to return too early.
> 
> To fix this, I propose reordering checks in `LoadNode::Value`.

Looks reasonable. Does the new node come from here?
https://github.com/openjdk/jdk/blob/f6e7713bb653811423eeb2515c2f69b437750326/src/hotspot/share/opto/memnode.cpp#L1211-L1215

Should we generally add some verification code that `Identity()` does not create new nodes? Hard to do it at all places but it could, for example, be done in the `transform()` methods of GVN and IGVN when calling `Identity()`. But of course, should probably be done in a separate RFE.

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22818#pullrequestreview-2514875543