RFR: 8343685: C2 SuperWord: refactor VPointer with MemPointer [v14]

[Christian Hagedorn]

On Sat, 18 Jan 2025 05:47:33 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/mempointer.hpp line 829:
>> 
>>> 827: };
>>> 828: 
>>> 829: class MemPointerParser : public StackObj {
>> 
>> Maybe a quick class comment to summarize the input and output could be helpful here.
>
> I wrote this, lmk if that is what you wanted:
> 
> // Utility class.
> // MemPointerParser::parse takes a MemNode (load or store) and computes its MemPointer.
> // It temporarily allocates dynamic data structures (GrowableArray) in the resource
> // area. This way, the computed MemPointer does not have to have any dynamic data
> // structures and can be copied freely by value.

Perfect :-)

>> src/hotspot/share/opto/superword.hpp line 568:
>> 
>>> 566:     MemNode* _mem;
>>> 567:     const VPointer* _vpointer;
>>> 568:     int _original_index;
>> 
>> Can also be made const:
>> Suggestion:
>> 
>>     const int _original_index;
>
> That does not work sadly:
> 
> In file included from /oracle-work/jdk-fork5/open/src/hotspot/share/classfile/classLoaderData.hpp:33,
>                  from /oracle-work/jdk-fork5/open/src/hotspot/share/precompiled/precompiled.hpp:34:
> /oracle-work/jdk-fork5/open/src/hotspot/share/utilities/growableArray.hpp: In instantiation of 'int GrowableArrayWithAllocator<E, Derived>::append(const E&) [with E = SuperWord::MemOp; Derived = GrowableArray<SuperWord::MemOp>]':
> /oracle-work/jdk-fork5/open/src/hotspot/share/opto/superword.cpp:566:20:   required from here
> /oracle-work/jdk-fork5/open/src/hotspot/share/utilities/growableArray.hpp:414:22: error: use of deleted function 'SuperWord::MemOp& SuperWord::MemOp::operator=(const SuperWord::MemOp&)'
>   414 |     this->_data[idx] = elem;
>       |     ~~~~~~~~~~~~~~~~~^~~~~~
> In file included from /oracle-work/jdk-fork5/open/src/hotspot/share/opto/superword.cpp:29:
> /oracle-work/jdk-fork5/open/src/hotspot/share/opto/superword.hpp:564:9: note: 'SuperWord::MemOp& SuperWord::MemOp::operator=(const SuperWord::MemOp&)' is implicitly deleted because the default definition would be ill-formed:
>   564 |   class MemOp : public StackObj {
>       |         ^~~~~
> /oracle-work/jdk-fork5/open/src/hotspot/share/opto/superword.hpp:564:9: error: non-static const member 'const int SuperWord::MemOp::_original_index', cannot use default assignment operator
> 
> It is a limitation with `GrowableArray`. But I think maybe more generally with sort: you need to be able to swap the values of different `MemOp`.

Ah ok, that's unfortunate.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21926#discussion_r1921913626
PR Review Comment: https://git.openjdk.org/jdk/pull/21926#discussion_r1921914019