RFR: 8332670: C1 clone intrinsic needs memory barriers

[Aleksey Shipilev]

On Tue, 4 Jun 2024 08:10:59 GMT, Galder Zamarreño <galder at openjdk.org> wrote:

> Adds a storestore barrier after copying the contents in the primitive array intrinsic (credit @shipilev). The barrier is a no-op in platforms where not needed so no need for an ifdef.
> 
> The barrier after new array creation is only added if zeroing the array on aarch64 (credit @dean-long). Since the primitive array clone intrinsic does not zero the array, that means there's a single barrier added for this use case.
> 
> There's no barrier added on x86 c1 macro assembler for nothing to do there. 
> 
> I've run the following tests:
> * tier 1 on darwin/aarch64
> * tier 1 on linux/x86_64
> * `hotspot_compiler` tests on darwin/aarch64
> * `copy.clone.arrays` jcstress tests on darwin/aarch64.
> 
> I tried but was unable to create a standalone test for the jdk source tree that would fail.
> 
> FYI @bulasevich @TheRealMDoerr @RealFYang @RealLucy similar platform specific c1 macro assembler changes might be required for other platforms.

All right, that looks reasonable.

I am a bit queasy on removing the storestore barrier from the allocation path, given that it also protects the object metadata. An accidentally missing barrier would probably lead to VM crash, that is in the best case. Current code paths do not seem to be affected by this, but there is also no guardrails that would protect us from making such a mistake in the future: someone adds `new NewTypeArray` somewhere, and forgets a trailing barrier? 

What would be the cost of still emitting (an excess for cloning path) StoreStore in `C1_MacroAssembler::allocate_array`? Would that cost still matter, given the performance improvement we get with C1 clone intrinsic?

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

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19538#pullrequestreview-2095739983