RFR: 8343685: C2 SuperWord: refactor VPointer with MemPointer [v6]
Vladimir Kozlov
kvn at openjdk.org
Wed Jan 15 07:19:34 UTC 2025
On Wed, 15 Jan 2025 06:43:23 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.**
>> See: https://eme64.github.io/blog/2025/01/01/AutoVectorization-Status.html
>>
>> 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: 31...
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>
> More fixes for vnkozlov
Thank you for answering all my questions.
Now we need to decide what to do with commented tests. It is not clear your intention what to do with them.
> "This one is array, and for some reason does not currently parse pointers sufficiently well to vectorize:"
Do you plan to fix it (in an other changes)? Then you need to say it in comment.
What happens with a test which rules are commented (as in your change)? Does it run or not? Does it ignore results if it runs? Why to run it then?
-------------
Marked as reviewed by kvn (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/21926#pullrequestreview-2551699684
PR Comment: https://git.openjdk.org/jdk/pull/21926#issuecomment-2591785779
More information about the hotspot-compiler-dev
mailing list