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

[Emanuel Peter]

On Wed, 27 Mar 2024 09:08:35 GMT, Christian Hagedorn <chagedorn 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
>
> Thanks for the updates. The PR generally looks good! I now fully reviewed it and left some more comments.

@chhagedorn thanks very much for the review, there were some great suggestions! I fixed almost all, and left some responses at the others.

I'll repeat them here:

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

You are right, I could actually remove the SuperWord reference from the PacksetGraph completely, and just pass the components, as const references. That would be really nice.

If it is ok for you, I will do this in a future RFE.

Actually, I plan to completely overhaul the PacksetGraph. It will be transformed to the VTransformGraph, and it will do:

Cycle checking (like today)
Evaluate the cost-model
Execute: each node knows how to replace its packed scalar nodes with vector nodes (basically refactoring SuperWord::output away)
etc.
We may even be able to take the VTransformGraph and try to widen all nodes, or make transformations on this graph, a simplified version of IGVN. I have lots of ideas that would be unlocked with this new graph-based approach.

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

We moved the implementation of split / filter to the packset, and that makes sense: we can hide the packset internals, and don't have them spill out to the SuperWord code. We can just call _packset.split_packs with a split_strategy. But defining the split strategy itself depends on other SuperWord components, and so I think conceptually they belong with SuperWord. They query reductions, dependency graph, implemented, AlignmentSolution, profitable, etc.
Sure, we can just pass a SuperWord reference, but that does not really seem right to me either. For me, the SuperWord class is there to manage the interface between all the components, and try to avoid passing components to other components, wherever possible. So I would rather have a list of methods in SuperWord, and each such method defines how the components interact (e.g. packset and alignment, packset and AlignmentSolution, pairset and packset, ...).
But we can discuss this further, and maybe come up with an even better solution. My hope is just that we separate the components as much as possible, so that we know that only a handful of them interact at a given point. That makes the whole beast more managable.

----------

Using `Pack` instead of `Node_List`. Great idea, would improve readability. Let's do it in a future RFE.

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

PR Comment: https://git.openjdk.org/jdk/pull/18276#issuecomment-2023112024