RFR: 8333791: Fix memory barriers for @Stable fields

[Chen Liang]

On Mon, 10 Jun 2024 18:05:09 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> See bug for more discussion.
> 
> Currently, C2 puts a `Release` barrier at exit of _every_ method that writes a `@Stable` field. This is a problem for high-performance code that initializes the stable field like this: https://github.com/openjdk/jdk/blob/79a23017fc7154738c375fbb12a997525c3bf9e7/src/java.base/share/classes/java/lang/Enum.java#L182-L193
> 
> A more egregious example is here, which means that every `String` constructor actually does `Release` barrier for `@Stable` field write, while only a `StoreStore` for `final` field store would suffice:
> https://github.com/openjdk/jdk/blob/79a23017fc7154738c375fbb12a997525c3bf9e7/src/java.base/share/classes/java/lang/String.java#L159-L160
> 
> AFAICS, the original intent for Release barrier in constructor for stable fields was to match the memory semantics of final fields better. `@Stable` are in some sense "super-finals": they are foldable like static finals or non-static trusted finals, but can be written anywhere. The `@Stable` machinery is intrinsically safe under races: either a compiler sees a component of stable subgraph in initialized state and folds it, or it sees a default value for the component and leaves it alone.
> 
> I [performed an audit](https://bugs.openjdk.org/browse/JDK-8333791?focusedId=14688000&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14688000) of current `@Stable` uses for fields that are not currently `final` or `volatile`, and there are cases where we write into `@Stable` fields in constructors. AFAICS, they are covered by final-field-like semantics by accident of having adjacent `final` fields.
> 
> Current PR implements Variant 2 from the discussion: makes sure stable fields are as memory-safe as finals, and that's it. I believe this is all-around a good compromise for both mainline and the backports: the performance is improved in one the path that matter, and we still have some safety margin in face of accidental removals of adjacent `final`-s, or in case I missed some spots during the audit.
> 
> C1 did not do anything special for `@Stable` fields at all, fixed those to match C2. Both Zero and template interpreters for non-TSO arches put barriers at every `return` (with notable exception of [ARM32](https://bugs.openjdk.org/browse/JDK-8333957)), which handles everything in an overkill manner.
> 
> Additional testing:
>  - [x] New IR tests
>  - [x] Linux x86_64 server fastdebug, `all`
>  - [x] Linux AArch64 server fastdebug, `all`
>  - [x] Linux AArch64 server fastdebug, jcstre...

Marked as reviewed by liach (Reviewer).

src/hotspot/share/opto/parse1.cpp line 1040:

> 1038:     if (PrintOpto && (Verbose || WizardMode)) {
> 1039:       method()->print_name();
> 1040:       tty->print_cr(" writes @Stable and needs a memory barrier");

This is the generic, non-constructor stable write release barrier removed, right?

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

PR Review: https://git.openjdk.org/jdk/pull/19635#pullrequestreview-2175116293
PR Review Comment: https://git.openjdk.org/jdk/pull/19635#discussion_r1676061372