RFR: 8318446: C2: optimize stores into primitive arrays by combining values into larger store [v11]

[Tobias Hartmann]

On Mon, 26 Feb 2024 19:25:01 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> @vnkozlov Hmm, ok. You seem to say that it is mostly a question of bugs that exist elsewhere. But my change here would not really change things then, since we could always trigger those things with Unsafe anyway, right?
>> 
>> Or is there something intrinsically wrong about using **raw store**?
>> 
>> Is there any alternative for me here? @TobiHartmann suggested I could use CPU barriers.
>
> Adding `MemBarCPUOrder` after this store is good suggestion. But hte need for it depends how you replace all merged stores with new wide store and attach following loads. Looking on changes and it seems you allow this optimization for sequential fields of normal object and not just array (I don't see check for AryPtrType). Such accesses will have separate memory slices (aliases). If you attach all memory edges of following related Load nodes to new wide Store we may be fine because loads will not pass the store. If not, we have a problem. It needs to be verified. Also looking on `find_previous_store()` and `can_see_stored_value()` they seem have checks which may prevent Loads from looking above such Store.
> 
> We need more testing for such cases. I see only 2 test methods which do that: test300R and test300a.
> Also all since all init values the same it is difficult to see if Load node get correct value.

Right, for sequential fields it's definitely a problem but I thought this optimization is limited to arrays? I hit this with a similar optimization in Valhalla and as a quick workaround, I'm using the `C2_UNSAFE_ACCESS` decorator for the accesses which will emit a membar via `C2Access::needs_cpu_membar()`. It's worth checking though if it is really needed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16245#discussion_r1503903271