RFR: 8287647: VM debug support: find node by pattern in name or dump
Christian Hagedorn
chagedorn at openjdk.java.net
Mon Jun 13 09:00:09 UTC 2022
On Thu, 2 Jun 2022 09:16:28 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
> **Goal**
> Refactor `Node::find`, allow not just searching for `node->_idx`, but also matching for `node->Name()` and even `node->dump()`.
>
> **Proposal**
> Refactor `Node::find` into `visit_nodes`, which visits all nodes and calls a `callback` on them. This callback can be used to filter by `idx` (`find_node_by_idx`, `Node::find`, `find_node` etc.). It can also be used to match node names (`find_node_by_name`) and even node dump (`find_node_by_dump`).
>
> Thus, I present these additional functions:
> `Node* find_node_by_name(const char* name)`: find all nodes matching the `name` pattern.
> `Node* find_node_by_dump(const char* pattern)`: find all nodes matching the `pattern`.
> The nodes are sorted by node idx, and then dumped.
>
> Patterns can contain `*` characters to match any characters (eg. `Con*L` matches both `ConL` and `ConvI2L`)
>
> **Usecase**
> Find all `CastII` nodes. Find all `Loop` nodes. Use `find_node_by_name`.
>
> Find all all `CastII` nodes that depend on a rangecheck. Use `find_node_by_dump("CastII*range check dependency")`.
> Find all `Bool` nodes that perform a `[ne]` check. Use `find_node_by_dump("Bool*[ne]")`.
> Find all `Phi` nodes that are `tripcount`. Use `find_node_by_dump("Phi*tripcount")`.
>
> Find all `Load` nodes that are associated with line 301 in some file. Use `find_node_by_dump("Load*line 301")`.
>
> You can probably find more usecases yourself ;)
Otherwise, looks good! These are useful new `find` methods to have!
src/hotspot/share/opto/node.cpp line 1623:
> 1621: return _worklist[i];
> 1622: }
> 1623: size_t size() {
You should add some new lines to better separate the methods.
src/hotspot/share/opto/node.cpp line 1627:
> 1625: }
> 1626: private:
> 1627: uint _index = 0;
Is not used anymore and can be removed.
src/hotspot/share/opto/node.cpp line 1703:
> 1701: char buf[N]; // copy parts of pattern into this
> 1702: const char* s = str;
> 1703: const char* r = &pattern[0]; // cast array to char*
You can directly use `pattern` which is the same as `&pattern[0]`
Suggestion:
const char* r = pattern;
Maybe you could also use a more descriptive name for `s` and `r`. Maybe `str_index` and `pattern_index`, respectively?
src/hotspot/share/opto/node.cpp line 1712:
> 1710: strncpy(buf, r, r_part_len);
> 1711: buf[r_part_len] = '\0'; // end of string
> 1712: r_part = &buf[0]; // cast array to char*
Suggestion:
r_part = buf;
-------------
Marked as reviewed by chagedorn (Reviewer).
PR: https://git.openjdk.org/jdk/pull/8988
More information about the hotspot-compiler-dev
mailing list