RFR: 8308994: C2: Re-implement experimental post loop vectorization [v2]

[Emanuel Peter]

On Mon, 3 Jul 2023 08:05:33 GMT, Pengfei Li <pli at openjdk.org> wrote:

>> src/hotspot/share/opto/superword.cpp line 179:
>> 
>>> 177:     assert(_packset.length() == 0, "packset must be empty");
>>> 178:     success = SLP_extract();
>>> 179:     if (PostLoopMultiversioning) {
>> 
>> Could we now have an assert for `cl->is_main_loop()` at the beginning of `SuperWord::transform_loop`, and remove all checks for it in SuperWord?
>
> Unfortunately, I just tried updating this but found assertion failures. I see `SuperWord::transform_loop()` is also called in `IdealLoopTree::policy_unroll_slp_analysis()` which can pass a normal loop (the loop before iteration-split). I assume only main loops require unrolling analysis and don't understand why it could be a normal loop. Maybe that's bad code and we need refactor C2's unrolling analysis first.

It would be great if we could untangle that a bit. Let me know what idea you come up with.
It also sounds confusing that the "analysis" only `policy_unroll_slp_analysis` should call a method that is called "transform" like `transform_loop`.

>> src/hotspot/share/opto/superword.hpp line 666:
>> 
>>> 664:   IdealLoopTree*  lpt() const   { return _lpt; }
>>> 665:   PhiNode* iv() const {
>>> 666:     return _slp ? _slp->iv() : _lpt->_head->as_CountedLoop()->phi()->as_Phi();
>> 
>> I'd suggest either cache it directly from `_lpt->_head->as_CountedLoop()->phi()->as_Phi()`, or just query it directly. Reduce dependence on `_slp`.
>
> Good catch! What do you think of getting rid of `_slp` completely in `SWPointer` refactoring?

I think that would be optimal, if it is possible. I would maybe call it a `CLPointer`, for counted-loop-pointer? And only have a reference to the `_lpt` / `cl`. Eventually, we may want to even allow non-conted-loops, but that is really for the future.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14581#discussion_r1250981304
PR Review Comment: https://git.openjdk.org/jdk/pull/14581#discussion_r1250984838