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

Emanuel Peter epeter at openjdk.org
Wed Mar 27 14:39:35 UTC 2024


On Tue, 26 Mar 2024 15:50:29 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
>
> src/hotspot/share/opto/superword.cpp line 1539:
> 
>> 1537: 
>> 1538: // Split packs at boundaries where left and right have different use or def packs.
>> 1539: void SuperWord::split_packs_at_use_def_boundaries() {
> 
> As above with `PairSet`, I'm also wondering here if these `split` methods could be part of `PackSet`? But I have not checked all the method calls and it could very well be that you would need to pass a reference to `SuperWord` to `PackSet`. From a high-level view, "split packs" feels like an operation you perform on a pack set. Same, for example, as `SuperWord::verify_packs()`. 
> 
> Either way, I think the patch is already quite big and I think we should do that - if wanted - separately

Yes, let's consider that for a future RFE.

But for now I can say this:
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.

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

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


More information about the hotspot-compiler-dev mailing list