RFR: 8373343: C2: verify AddP base input only set for heap addresses [v3]

[Christian Hagedorn]

On Fri, 19 Dec 2025 13:15:38 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> The base input of `AddP` is expected to only be set for heap accesses
>> but I noticed some inconsistencies so I added an assert in the `AddP`
>> constructor and fixed issues that it caught. AFAFICT, the
>> inconsistencies shouldn't create issues.
>
> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review

Good improvement! I have two more suggestions but these could also be done separately.

src/hotspot/share/opto/graphKit.cpp line 2737:

> 2735: 
> 2736:   // First load the super-klass's check-offset
> 2737:   Node *p1 = gvn.transform(new AddPNode(C->top(), superklass, gvn.MakeConX(in_bytes(Klass::super_check_offset_offset()))));

Would it make sense to have 2 constructors or even better 2 `make()` methods with dedicated names instead of passing in top each time?

For example:

make_with_base(Node* base, Node* ptr, Node* offset)
make_off_heap(Node* ptr, Node* offset)

`make_off_heap()` can then call `make_with_base(Compile::current()->top, ptr, offset)`

src/hotspot/share/opto/graphKit.cpp line 2755:

> 2753:   chk_off_X = gvn.transform(new ConvI2LNode(chk_off_X));
> 2754: #endif
> 2755:   Node *p2 = gvn.transform(new AddPNode(C->top(),subklass,chk_off_X));

Suggestion:

  Node* p2 = gvn.transform(new AddPNode(C->top(), subklass, chk_off_X));

src/hotspot/share/opto/graphKit.cpp line 3590:

> 3588:   }
> 3589:   constant_value = Klass::_lh_neutral_value;  // put in a known value
> 3590:   Node* lhp = basic_plus_adr(top(), klass_node, in_bytes(Klass::layout_helper_offset()));

Same thought here: could we have a separate `off_heap_plus_addr()` or something like that instead of passing in `top()` on each call site?

This and the other suggestion could also be done separately.

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/28769#pullrequestreview-3602937227
PR Review Comment: https://git.openjdk.org/jdk/pull/28769#discussion_r2638913566
PR Review Comment: https://git.openjdk.org/jdk/pull/28769#discussion_r2638921027
PR Review Comment: https://git.openjdk.org/jdk/pull/28769#discussion_r2638920830