RFR: 8374789: C2: refactor GraphKit code that create AddP nodes for raw memory to use helper method [v3]

[Christian Hagedorn]

On Thu, 12 Feb 2026 13:25:39 GMT, Guanqiang Han <ghan at openjdk.org> wrote:

>> This PR refactors GraphKit code that creates AddP nodes for raw memory to use a dedicated helper method. This avoids passing top() explicitly at call sites and improves readability.
>> 
>> Please review this change. Thanks!
>> 
>> **Test:** GHA
>
> Guanqiang Han has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Align properly

Thanks for doing the additional changes! I have some more comments.

src/hotspot/share/opto/addnode.cpp line 793:

> 791: Node *AddPNode::Ideal(PhaseGVN *phase, bool can_reshape) {
> 792:   // Bail out if dead inputs
> 793:   if (phase->type(in(Address)) == Type::TOP) return nullptr;

I'm generally in favor of clean-ups but you touched quite some places already and doing the additional clean-ups further away in the same methods feels like being too much. I suggest to only stick to the actual refactoring of creating `AddP` nodes and possibly follow-up with more RFEs for other clean-ups if necessary. For the directly touched code, you are encouraged to change code style and maybe some immediately surrounded lines if you see fit. Btw, you can also change the `*` position from `Node *n` to `Node* n`.

src/hotspot/share/opto/addnode.hpp line 225:

> 223: class AddPNode : public Node {
> 224: private:
> 225:   AddPNode(Node *base, Node *ptr, Node *off) : Node(nullptr,base,ptr,off) {

While touching this, you can also fix the code style:

Suggestion:

  AddPNode(Node* base, Node* ptr, Node* off) : Node(nullptr, base, ptr, off) {

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

> 1184: Node* GraphKit::off_heap_plus_addr(Node* ptr, Node* offset) {
> 1185:   // short-circuit a common case
> 1186:   if (offset == intcon(0))  return ptr;

You should add braces for good measure:
Suggestion:

  if (offset == intcon(0)) { return ptr; }

Here, I think you can also apply the same to `basic_plus_adr()` since you copied that part from there.

src/hotspot/share/opto/graphKit.hpp line 326:

> 324:     return off_heap_plus_addr(ptr, MakeConX(offset));
> 325:   }
> 326:   Node* off_heap_plus_addr(Node* ptr, Node* offset);

Suggestion:

  }
  
  Node* off_heap_plus_addr(Node* ptr, Node* offset);

src/hotspot/share/opto/macro.cpp line 277:

> 275:     assert(ac->in(ArrayCopyNode::Src) != ac->in(ArrayCopyNode::Dest), "clone source equals destination");
> 276:     Node* base = ac->in(ArrayCopyNode::Src);
> 277:     Node* adr = _igvn.transform(AddPNode::make_with_base(base, base, _igvn.MakeConX(offset)));

We could also provide an overloaded `make_with_base` that only takes a `base` (i.e. where `base` == `ptr`) to avoid repeating the args. There are quite some occurrences of this pattern.

src/hotspot/share/opto/macro.cpp line 1951:

> 1949:       for (intx i = 0; i < lines; i++) {
> 1950:         prefetch_adr = AddPNode::make_off_heap(new_eden_top,
> 1951:                                             _igvn.MakeConX(distance));

Suggestion:

                                               _igvn.MakeConX(distance));

src/hotspot/share/opto/macro.hpp line 58:

> 56: 
> 57:   Node* off_heap_plus_addr(Node* ptr, int offset) {
> 58:     return (offset == 0)? ptr: off_heap_plus_addr(ptr, MakeConX(offset));

Suggestion:

    return (offset == 0) ? ptr : off_heap_plus_addr(ptr, MakeConX(offset));

src/hotspot/share/opto/macro.hpp line 60:

> 58:     return (offset == 0)? ptr: off_heap_plus_addr(ptr, MakeConX(offset));
> 59:   }
> 60:   Node* off_heap_plus_addr(Node* ptr, Node* offset) {

Suggestion:

  }

  Node* off_heap_plus_addr(Node* ptr, Node* offset) {

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

PR Review: https://git.openjdk.org/jdk/pull/29666#pullrequestreview-3808464276
PR Review Comment: https://git.openjdk.org/jdk/pull/29666#discussion_r2812356727
PR Review Comment: https://git.openjdk.org/jdk/pull/29666#discussion_r2812295032
PR Review Comment: https://git.openjdk.org/jdk/pull/29666#discussion_r2812284884
PR Review Comment: https://git.openjdk.org/jdk/pull/29666#discussion_r2812362077
PR Review Comment: https://git.openjdk.org/jdk/pull/29666#discussion_r2812326444
PR Review Comment: https://git.openjdk.org/jdk/pull/29666#discussion_r2812334181
PR Review Comment: https://git.openjdk.org/jdk/pull/29666#discussion_r2812279629
PR Review Comment: https://git.openjdk.org/jdk/pull/29666#discussion_r2812276997