RFR: 8349835: C2: simplify IGV property printing [v2]

Christian Hagedorn chagedorn at openjdk.org
Wed Oct 22 10:39:06 UTC 2025


On Wed, 1 Oct 2025 12:28:38 GMT, Saranya Natarajan <snatarajan at openjdk.org> wrote:

>> The code that prints node properties and live range properties is very verbose and repetitive and could be simplified by applying a refactoring suggested [here](https://github.com/openjdk/jdk/pull/23558#discussion_r1950785708).
>> 
>> ### Fix 
>> Implemented the suggested refactoring. 
>> 
>> ### Testing 
>> Github Actions, Tier 1-3
>
> Saranya Natarajan has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - fixing test failure
>  - addressing review comments

Thanks for the update, that looks much better! Some more small follow-up comments.

src/hotspot/share/opto/idealGraphPrinter.cpp line 1114:

> 1112: }
> 1113: 
> 1114: void PrintProperties::print_node_properties(Node* node, Compile* C){

Suggestion:

void PrintProperties::print_node_properties(Node* node, Compile* C) {

src/hotspot/share/opto/idealGraphPrinter.cpp line 1180:

> 1178:   if (flag) {
> 1179:     _printer->print_prop(name, val);
> 1180:   }

We should not use implicit conversion of ints, same below:


Suggestion:

  if (flag != 0) {
    _printer->print_prop(name, IdealGraphPrinter::TRUE_VALUE);
  }
}

void PrintProperties::print_property(int flag, const char* name, const char* val) {
  if (flag != 0) {
    _printer->print_prop(name, val);
  }
}

void PrintProperties::print_property(int flag, const char* name, int val) {
  if (flag != 0) {
    _printer->print_prop(name, val);
  }

src/hotspot/share/opto/idealGraphPrinter.hpp line 34:

> 32: #include "utilities/xmlstream.hpp"
> 33: 
> 34: 

Suggestion:

src/hotspot/share/opto/idealGraphPrinter.hpp line 172:

> 170: };
> 171: 
> 172: class PrintProperties

Do you really need it in the header file? You could also just move it the the source file directly where we use the class.

src/hotspot/share/opto/idealGraphPrinter.hpp line 175:

> 173: {
> 174: private:
> 175:   IdealGraphPrinter *_printer;

For new code, we should put the the `*` at the type.

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

PR Review: https://git.openjdk.org/jdk/pull/26902#pullrequestreview-3364567310
PR Review Comment: https://git.openjdk.org/jdk/pull/26902#discussion_r2451131733
PR Review Comment: https://git.openjdk.org/jdk/pull/26902#discussion_r2451184881
PR Review Comment: https://git.openjdk.org/jdk/pull/26902#discussion_r2451116971
PR Review Comment: https://git.openjdk.org/jdk/pull/26902#discussion_r2451120931
PR Review Comment: https://git.openjdk.org/jdk/pull/26902#discussion_r2451558310


More information about the hotspot-compiler-dev mailing list