RFR: 8286179: Node::find(int) should not traverse from new to old nodes

[Vladimir Kozlov]

On Fri, 6 May 2022 08:24:26 GMT, Emanuel Peter <duke at openjdk.java.net> wrote:

> **Problem:**
> `Node::find` traverses input and output edges of nodes during its BFS, and searches for nodes with a specific `idx`.
> However, if `ASSERT` is on, it also traverses `debug_orig`. This not only seems unnecessary. But Mach nodes (after matching) point back to the old IR nodes. This means we traverse from the new graph to the old graph, and potentially find multiple nodes matching the `idx`. Only the last found will be returned, sometimes this happens to be the new node, sometimes the old node. This is inconsistent and can be quite annoying during debugging.
> 
> **Implemented Solution:**
> 1. Remove traversing `debug_orig`.
> 2. Instead, add debug only functions `old_root`, which finds the old root if it exists. Question: I now put a warning in if the `old_root` cannot be found. I think this is helpful for in the debugger. I could make it an assert if that is preferred.
> 3. `find_node` and `find_ctrl` only search in new nodes now (start BFS at new root).
> 4. Added `find_old_node` and `find_old_ctrl`, which search in new nodes (start BFS at old root).
> 
> I hope this improves your debugging experience.
> [running sanity tests to see it doesn't break something]

Good. I have 2 comments.

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

> 1614: // Call this from debugger, search in old nodes:
> 1615: Node* find_old_node(const int idx) {
> 1616:   return old_root()->find(idx);

Need check for `nullptr` for `old_root()` call and may be do nothing since we will get message already.

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

> 1629: // Call this from debugger, search in old nodes:
> 1630: Node* find_old_ctrl(const int idx) {
> 1631:   return old_root()->find_ctrl(idx);

Need `nullptr` check.

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

PR: https://git.openjdk.java.net/jdk/pull/8567