RFR: 8332163: C2 SuperWord: refactor PacksetGraph and SuperWord::output into VTransformGraph [v11]

[Emanuel Peter]

On Thu, 4 Jul 2024 08:23:38 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   rm newline
>
> src/hotspot/share/opto/vtransform.hpp line 266:
> 
>> 264:     _req(req),
>> 265:     _in(vtransform.arena(),  req, req, nullptr),
>> 266:     _out(vtransform.arena(), 4, 0, nullptr)
> 
> Just noticed this here and totally unrelated, but there are a lot of places where we create a `GrowableArray` without a filler. So, we just pass `nullptr` or 0 (latter should probably be replaced by `nullptr`). Maybe `GrowableArray` should provide a constructor without a filler. Might be worth to do in an RFE at some point.

Feel free to take this up with the Runtime team ;)

> src/hotspot/share/opto/vtransform.hpp line 336:
> 
>> 334:     VTransformNode(vtransform, n->req()), _node(n) {}
>> 335:   Node* node() const { return _node; }
>> 336:   virtual VTransformScalarNode* isa_Scalar() override { return this; }
> 
> General comment about vtnode classes: The IDE suggests here and at other places that:
> 
> Clang-Tidy: 'virtual' is redundant since the function is already declared 'override'
> 
> So, you could get rid of `virtual` wherever you have `override`.

I like seeing that it is virtual. `override` is not consistently used in our code-base, but everything is called `virtual`. So I'd like to keep the `virtual` explicit.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19719#discussion_r1665479911
PR Review Comment: https://git.openjdk.org/jdk/pull/19719#discussion_r1665483501