RFR: 8350485: C2: factor out common code in Node::grow() and Node::out_grow()

Emanuel Peter epeter at openjdk.org
Mon Mar 10 10:58:27 UTC 2025


On Thu, 6 Mar 2025 09:08:41 GMT, Saranya Natarajan <duke at openjdk.org> wrote:

> Node:grow() and Node::out_grow() are copy-pasted from each other and their core logic could be factored out into a third function or at least cleaned up. Hence,the fix includes a function Node::array_resize() that implements the core logic of Node::grow() and Node::out_grow(). 
> 
>  Link to Github action which had no failures :  https://github.com/sarannat/jdk/actions/runs/13677508359

@sarannat I'm mostly leaving code style comments. I promise I won't be so pedantic in the future ;)

It may be good for you to read through this, at least to have an overview:
https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md

src/hotspot/share/opto/node.cpp line 667:

> 665: }
> 666: 
> 667: //------------------------------resize_array-------------------------------------------

Suggestion:


Just FYI. You can have these headers, but it is not expected any more. So I usually leave them out. But up to you.

src/hotspot/share/opto/node.cpp line 670:

> 668:  // Resize input or output array to grow it the next larger power-of-2 bigger
> 669: // than len.
> 670: void Node::resize_array( Node**& array, node_idx_t& max_size, uint len, bool is_in) {

Suggestion:

void Node::resize_array(Node**& array, node_idx_t& max_size, uint len, bool is_in) {

Code style

src/hotspot/share/opto/node.cpp line 673:

> 671:   Arena* arena = Compile::current()->node_arena();
> 672:   uint new_max = max_size;
> 673:   if( new_max == 0 ) {

Suggestion:

  if(new_max == 0) {

While we are here we might as well fix this too.

src/hotspot/share/opto/node.cpp line 675:

> 673:   if( new_max == 0 ) {
> 674:     max_size = 4;
> 675:     array = (Node**)arena->Amalloc(4*sizeof(Node*));

Suggestion:

    array = (Node**)arena->Amalloc(4 * sizeof(Node*));

We generally want to have spaces around operators.

See:
https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md

> Use spaces around operators, especially comparisons and assignments. (Relaxable for boolean expressions and high-precedence operators in classic math-style formulas.)

src/hotspot/share/opto/node.cpp line 683:

> 681:     }
> 682:       return;
> 683:   }

Suggestion:

    }
    return;
  }

Indentation

src/hotspot/share/opto/node.hpp line 340:

> 338:   // Resize input or output array to grow it the next larger power-of-2 bigger
> 339:   // than len.
> 340:   void resize_array(Node **&array, node_idx_t &max_size, uint len, bool is_in);

Suggestion:

  void resize_array(Node**& array, node_idx_t &max_size, uint len, bool is_in);

Nit code style ;)

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23928#pullrequestreview-2667282496
PR Review Comment: https://git.openjdk.org/jdk/pull/23928#discussion_r1985077851
PR Review Comment: https://git.openjdk.org/jdk/pull/23928#discussion_r1985076647
PR Review Comment: https://git.openjdk.org/jdk/pull/23928#discussion_r1985078681
PR Review Comment: https://git.openjdk.org/jdk/pull/23928#discussion_r1985080375
PR Review Comment: https://git.openjdk.org/jdk/pull/23928#discussion_r1985080970
PR Review Comment: https://git.openjdk.org/jdk/pull/23928#discussion_r1985075533


More information about the hotspot-compiler-dev mailing list