RFR: 8343685: C2 SuperWord: refactor VPointer with MemPointer [v3]

[Roland Westrelin]

On Fri, 13 Dec 2024 06:30:15 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> **This is a required step towards adding runtime-checks for Aliasing Analysis, especially Important for FFM / MemorySegments.**
>> 
>> I know this one is large, but it consists of a lot of renamings, and new tests. On the whole, the new `VPointer` code is less than the old!
>> 
>> **Goal**
>> 
>> Replace old `VPointer` with a new version that relies on `MemPointer` - which then is a shared utility for both `MergeStores` and `SuperWord / AutoVectorization`. `MemPointer` generally parses pointers, and `VPointer` specializes this facility for the use in loops (`VLoop`).
>> 
>> The old `VPointer` implementation with its recursive pattern matching was quite complicated and difficult to reason about for correctness. The approach in `MemPointer` is much simpler: iteratively decomposing sub-expressions. Further: the new implementation is more powerful at detecting equivalent invariants.
>> 
>> **Future**:  with the `MemPointer` implementation of `VPointer`, it should be easier to implement speculative runtime-checks for Aliasing-Analysis [JDK-8324751](https://bugs.openjdk.org/browse/JDK-8324751). The pressing need for this has come from the FFM / MemorySegment folks, like @mcimadamore and @minborg .
>> 
>> **Details**
>> 
>> This looks like a rather big patch, so let me explain the parts.
>> - Refactor of `MemPointer` in `mepointer.hpp/cpp`:
>>   - Added concept of `Base` to `MemPointer`. This is required for the aliasing computation in `VPointer`.
>>   - `sub_expression_has_native_base_candidate`: add special case to parse through `CastX2P` if we find a native memory base `MemorySegment.address()`, i.e. `jdk.internal.foreign.NativeMemorySegmentImpl.min`. This helps some native memory segment cases to vectorize that did not before.
>>   - So far `MemPointer` could only answer adjacency queries. But VPointer also needs overlap queries, see the old `VPointer::not_equal` (i.e. can we prove that the two `VPointer` never overlap?). So I had to add a new case to aliasing computation: `NotOrAtDistance`. It is useful to answer the new and better named `MemPointer::never_overlaps_with`.
>>   - Collapsed together `MemPointerDecomposedForm` and `MemPointer`. It was an unnecessary and unhelpful split.
>> - Re-write of `VPointer` based on `MemPointer`:
>>   - Old pattern:
>>     - `VPointer[mem:  847     StoreI, base:   37, adr:   37,  base[  37] + offset(  16) + invar(   0) + scale(   4) * iv]`
>>     - `VPointer[mem: 3189      LoadB, base:    1, adr: 2273,  base[   1] + offset(   0) + invar(   0...
>
> Emanuel Peter has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 114 commits:
> 
>  - manual merge
>  - fix printing
>  - rename
>  - fix up print
>  - add TestEquivalentInvariants.java
>  - improve documentation
>  - hide parser via delegation
>  - Merge branch 'master' into JDK-8343685-VPointer-MemPointer
>  - make sort stable
>  - some comment and naming improvements
>  - ... and 104 more: https://git.openjdk.org/jdk/compare/31ceec7c...4b0504d0

This is tricky to review but looks reasonable to me.

src/hotspot/share/opto/mempointer.cpp line 253:

> 251: // (2) LoadL from field jdk.internal.foreign.NativeMemorySegmentImpl.min
> 252: //     Holds the address() of a native memory segment.
> 253: bool MemPointerParser::is_native_memory_base_candidate(Node* n) {

Does this really belong to the move from VPointer to MemPointer? AFAIU, it's an extra optimization on top of the move. Should be done as a separate PR?

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

PR Review: https://git.openjdk.org/jdk/pull/21926#pullrequestreview-2502259410
PR Review Comment: https://git.openjdk.org/jdk/pull/21926#discussion_r1883911898