RFR: 8333791: Fix memory barriers for @Stable fields

[John R Rose]

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`

I think we should remove `wrote_stable` and all associated logic. The full argument is in my comment the bug.

https://bugs.openjdk.org/browse/JDK-8333791

Stable fields are in some ways “better finals”, in that they can be used to store lazy but effectively final states.  But part of the “better” is that (correctly used) their race conditions are safe.  Since racing is part of their nature, the fences are an unnecessary expense.

So just removing that code would be the best outcome, unless I am missing something.  We will want to run such a change through heavy testing.

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

PR Comment: https://git.openjdk.org/jdk/pull/19635#issuecomment-2161140122