RFR: 8312332: C2: Refactor SWPointer out from SuperWord [v3]

[Vladimir Kozlov]

On Wed, 30 Aug 2023 11:28:56 GMT, Pengfei Li <pli at openjdk.org> wrote:

>> As discussed in JDK-8308994, we should first do some refactoring work before proceeding with the new post loop vectorization. In this patch, we have done the following.
>> 
>> 1) We have created new C2 source files `vectorization.[cpp|hpp]` for shared logics and utilities for C2's auto-vectorization. So far we have moved class `SWPointer` and `VectorElementSizeStats` here from `superword.[cpp|hpp]`.
>> 
>> 2) We have decoupled `SWPointer` from class `SuperWord` and renamed it to `VPointer` as it will be used by vectorizers other than SuperWord. The original class `SWPointer` and its inner class `Tracer` both have a `_slp` field initialized in their constructors. In this patch, we have replaced them by other fields and re-written the constructors for the same functionality. Original `SWPointer::invariant()` calls function `SuperWord::find_pre_loop_end()` for loop invariant checks. To help decoupling, we moved function `find_pre_loop_end()` to class `CountedLoopNode`. As function `SWPointer::Tracer::invariant_1()` is tightly coupled with `SuperWord` but only prints some debug messages, we temporarily removed it in this patch. We will consider adding it back after later refactoring of `SuperWord` so we added a `TODO` at its call site in this patch.
>> 
>> 3) We have a lot of memory phi node checks in loop optimizations. So we added a utility function `is_memory_phi()` in `node.hpp`.
>> 
>> Tested tier1~3 on x86 and AArch64. Also manually verified that option `VectorizeDebug` in compiler directives still works well.
>
> Pengfei Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Move the cache of _pre_loop_end to CountedLoopNode

In general it looks good. I just want that you add more description.

And I have question. Part of code is under "#ifndef PRODUCT". Did you build "optimized" VM to make sure it works?

src/hotspot/share/opto/vectorization.hpp line 31:

> 29: #include "opto/loopnode.hpp"
> 30: 
> 31: // ----------------------------------VPointer----------------------------------

We stop using such lines - there was cleaning RFE to remove them.

src/hotspot/share/opto/vectorization.hpp line 32:

> 30: 
> 31: // ----------------------------------VPointer----------------------------------
> 32: // Information about an address for dependence checking and vector alignment

I am fine with using VPointer name but I think you need to add more description here to avoid confusion.

May be add a general description what is in this file: "for shared logics and utilities for C2's auto-vectorization".

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

PR Review: https://git.openjdk.org/jdk/pull/15013#pullrequestreview-1622675042
PR Review Comment: https://git.openjdk.org/jdk/pull/15013#discussion_r1323312102
PR Review Comment: https://git.openjdk.org/jdk/pull/15013#discussion_r1323299455