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