RFR: 8335392: C2 MergeStores: enhanced pointer parsing

[Emanuel Peter]

On Thu, 15 Aug 2024 14:34:18 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> **Background**
>> I am introducing the `MemPointer`, for enhanced pointer parsing. For now, it replaces the much more limited `ArrayPointer` in `MergeStores` (see https://github.com/openjdk/jdk/pull/16245), but eventually it is supposed to be used widely in optimizations for pointer analysis: adjacency, aliasing, etc. I also plan to refactor the `VPointer` from auto-vectorization with it, and unlock more pointer patterns that way - possibly including scatter/gather.
>> 
>> **Details**
>> 
>> The `MemPointer` decomposes a pointer into the form `pointer = con + sum_i(scale_i * variable_i)` - a linear form with a sum of variables and scale-coefficients, plus some constant offset.
>> 
>> This form allows us to perform aliasing checks - basically we can check if two pointers are always at a constant offset. This allows us to answer many questions, including if two pointers are adjacent. `MergeStores` needs to know if two stores are adjacent, so that we can safely merge them.
>> 
>> More details can be found in the description in `mempointer.hpp`. Please read them when reviewing!
>> 
>> `MemPointer` is more powerful than the previous `ArrayPointer`: the latter only allows arrays, the former also allows native memory accesses, `Unsafe` and `MemorySegement`.
>> 
>> **Dealing with Overflows**
>> 
>> We have to be very careful with overflows when dealing with pointers. For this, I introduced a `NoOverflowInt`. It allows us to do "normal" int operations on it, and tracks if there was ever an overflow. This way, we can do all overflow checks implicitly, and do not clutter the code with overflow-checks or - God forbid - forget overflow-checks.
>
> src/hotspot/share/opto/mempointer.cpp line 114:
> 
>> 112:         // Decompose subtraction.
>> 113:         Node* a = n->in((opc == Op_AddP) ? 2 : 1);
>> 114:         Node* b = n->in((opc == Op_AddP) ? 3 : 2);
> 
> rm AddP

Good catch, will remove it here.

> src/hotspot/share/opto/mempointer.cpp line 263:
> 
>> 261:   // Compute distance:
>> 262:   NoOverflowInt distance = other.con() - con();
>> 263:   distance = distance.truncate_to_30_bits();
> 
> naming could be an issue, why coud it be NaN

Ok, I now replaced it with a `bool` method `is_abs_less_than_2_to_30`. I think that is more clear.

> src/hotspot/share/opto/traceMergeStoresTag.hpp line 31:
> 
>> 29: #include "utilities/stringUtils.hpp"
>> 30: 
>> 31: namespace TraceMergeStores {
> 
> can this be a class?

Not easily. Without the namespace, I get a clash with `traceAutoVectorization.hpp` names. A class could work, but then I need to initialize the arrays elsewhere - probably in a new cpp file. Annoying.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19970#discussion_r1718560644
PR Review Comment: https://git.openjdk.org/jdk/pull/19970#discussion_r1718587908
PR Review Comment: https://git.openjdk.org/jdk/pull/19970#discussion_r1718601006