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