RFR: 8288897: Clean up node dump code [v4]
Emanuel Peter
epeter at openjdk.org
Fri Jul 8 15:47:18 UTC 2022
On Fri, 24 Jun 2022 09:20:43 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:
>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>>
>> implementing Christians review suggestions
>
> src/hotspot/share/opto/node.cpp line 2205:
>
>> 2203: }
>> 2204:
>> 2205: bool PrintBFS::filter_category(const Node* n, Filter& filter) {
>
> Maybe you could add a method comment that you are not filtering on the category for `Mixed` but actually look at the outputs of it and also consider `is_CFG()`.
done
> src/hotspot/share/opto/node.cpp line 2220:
>
>> 2218: }
>> 2219: if (filter._other && t->has_category(Type::Category::Other)) {
>> 2220: return true;
>
> Just a suggestion: To make it clear that you are only special casing `Mixed` you could leave the `switch` statement and only do the additional checks for `Mixed`. Since this category check is specific to the filtering of `dump_bfs()` and not something you normally perform on a type, I suggest to move this function to the `Filter` class (if that's possible). This would also require to change the implementation of `has_category()` - if it's too complicated, just leave it as it is. It's fine like that.
I now made a single if statement out of it, and moved it to `Filter`. It was not evident for me how to use `switch,` because both the filter and the node can have multiple categories.
-------------
PR: https://git.openjdk.org/jdk/pull/9234
More information about the hotspot-compiler-dev
mailing list