RFR: 8325252: C2 SuperWord: refactor the packset

[Emanuel Peter]

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

>> I'm refactoring the packset, separating the details of packset-manupulation from the SuperWord algorithm.
>> 
>> Most importantly: I split it into two classes: `PairSet` and `PackSet`.
>> `combine_pairs_to_longer_packs` converts the first into the second.
>> 
>> I was able to simplify the combining, and remove the pack-sorting.
>> I now walk "pair-chains" directly with `PairSetIterator`. One such pair-chain is equivalent to a pack.
>> 
>> I moved all the `filter / split` functionality to the `PackSet`, which allows hiding a lot of packset-manipulation from the SuperWord algorithm.
>> 
>> I ran into some issues when I was extending the pairset in `extend_pairset_with_more_pairs_by_following_use_and_def`:
>> Using the PairSetIterator changed the order of extension, and that messed with the packing heuristic, and quite a few examples did not vectorize, because we would pack up the wrong 2 nodes out of a choice of 4 (e.g. we would pack `ac bd` instead of `ab cd`). Hence, I now still have to keep the insertion order for the pairs, and this basically means we are extending with a BFS order. Maybe this issue can be removed, if I improve the packing heuristic with some look-ahead expansion approach (but that is for another day [JDK-8309908](https://bugs.openjdk.org/browse/JDK-8309908)).
>> 
>> But since I already spent some time on some of the packing heuristic (reordering and cost estimate), I did a light refactoring, and added extra tests for MulAddS2I.
>> 
>> More details are described in the annotations in the code.
>
> src/hotspot/share/opto/superword.cpp line 1253:
> 
>> 1251:   // 1. Reduction
>> 1252:   if (is_marked_reduction(use1) && is_marked_reduction(use2)) {
>> 1253:     Node* first = use1->in(2);
> 
> Note: while this refactoring here was not strictly necessary, I still took the time to improve the comments a bit. I needed to understand this anyway for the `_race_possible` removal.

Note: I also added some more cases for MulAddS2I test, reflecting some of the swaps in this method.

> src/hotspot/share/opto/superword.hpp line 226:
> 
>> 224: };
>> 225: 
>> 226: class PackSet : public StackObj {
> 
> Note: new class.

Nice about this new class is that I can now make some methods `private`, e.g.:
- `set_pack` (used to be `set_my_pack`)
- `split_pack`, is used inside public method `split_packs`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18276#discussion_r1531987178
PR Review Comment: https://git.openjdk.org/jdk/pull/18276#discussion_r1525028944