RFR: 8325252: C2 SuperWord: refactor the packset [v4]

[Christian Hagedorn]

On Thu, 14 Mar 2024 14:44:55 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   use left/right instead of s1/s2 in some obvious simple places
>
> src/hotspot/share/opto/superword.cpp line 1369:
> 
>> 1367:         left = right;
>> 1368:       }
>> 1369:       _packset.add_pack(pack);
> 
> Note: I replaced a quadratic loop, which basically checked all-with-all pairs, if they can be combined.
> An additional benefit: we don't need the packs sorted (this also removes the need to have all nodes annotated with alignment, but that will be more useful in a future RFE).

That's great!

> src/hotspot/share/opto/superword.hpp line 101:
> 
>> 99:   int length() const { return _lefts_in_insertion_order.length(); }
>> 100:   Node* left_at(int i)  const { return _body.body().at(_lefts_in_insertion_order.at(i)); }
>> 101:   Node* right_at(int i) const { return _body.body().at(get_right_for(_lefts_in_insertion_order.at(i))); }
> 
> Note: I hope to get rid of `_lefts_in_insertion_order` eventually, and then these accessors will disappear too. But for now I need them to iterate in insertion order, doing something similar to a DFS in the pair extension.

Make sense but maybe you can rename them `left/right_at_in_insertion_order()` to avoid misuses? Then you could name the `has_left/right(int i)` -> `left/right_at()`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18276#discussion_r1539502304
PR Review Comment: https://git.openjdk.org/jdk/pull/18276#discussion_r1540668934