RFR: 8288897: Clean up node dump code [v4]

Xin Liu xliu at openjdk.org
Thu Jul 14 07:07:07 UTC 2022


On Fri, 8 Jul 2022 15:47:17 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> I recently did some work in the area of `Node::dump` and `Node::find`, see [JDK-8287647](https://bugs.openjdk.org/browse/JDK-8287647) and [JDK-8283775](https://bugs.openjdk.org/browse/JDK-8283775).
>> 
>> This change sets cleans up the code around, and tries to reduce code duplication.
>> 
>> Things I did:
>> - remove Node::related. It was added 7 years ago, with [JDK-8004073](https://bugs.openjdk.org/browse/JDK-8004073). However, it was not extended to many nodes, and hence it is incomplete, and nobody I know seems to use it.
>> - refactor `dump(int)` to use `dump_bfs` (reduce code duplication).
>> - redefine categories in `dump_bfs`, focusing on output types. Mixed type is now also control if it has control output, and memory if it has memory output, etc. Plus, a node is also in the control category if it `is_CFG`. This makes `dump_bfs` much more usable, to traverse control and memory flow.
>> - Other small cleanups, like replacing rarely used dump functions with dump, making removing dead code, make some functions private
>> - Adding `call from debugger` comment to VM functions that are useful in debugger
>> - rename `find_node_by_name` to `find_nodes_by_name` and `find_node_by_dump` to `find_nodes_by_dump`.
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   implementing Christians review suggestions

nice feature!
LGTM. I am not a reviewer. we still needs other reviewers to approve it.

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

> 2077:   tty->print("      @: print old nodes - before matching (if available)\n");
> 2078:   tty->print("      B: print scheduling blocks (if available)\n");
> 2079:   tty->print("      $: dump only, no header, no other columns\n");

why don't we just list those options in alphabetical order? 
It's a little bit easier to interpret the default value 'cdmxo at B'  if we  flip order 'print m:... ' and 'print d:...' above.

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

> 2669: //   only_data:     whether to regard data edges only during traversal.
> 2670: static void collect_nodes_i(GrowableArray<Node*>* queue, const Node* start, int direction, uint depth, bool include_start, bool only_ctrl, bool only_data) {
> 2671:   bool indent = depth <= PrintIdealIndentThreshold;

hi, @eme64 , 
Could you also delete PrintIdealIndentThreshold from c2_globals.hpp?
This is like a hack to let node::dump() indent. I don't think it's quite useful.  it can be done in gdb pretty-print

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

Marked as reviewed by xliu (Committer).

PR: https://git.openjdk.org/jdk/pull/9234


More information about the hotspot-compiler-dev mailing list