RFR(M): 8029101: PPC64 (part 211): ordering of Independent Reads of Independent Writes

[Vitaly Davidovich]

Hi Goetz,

Volatile fields written in constructor aren't guaranteed by JMM to occur
before the reference is assigned; this guarantee only applies to final
fields and StoreStore (in JMM parlance) before ref assignment should be
sufficient.  Of course making the implementation stronger is probably ok,
but just wanted to point that out as there seem to be changes aimed at
this, as you say.

The reason volatile is excluded is because the ref assignment following the
volatile store is a plain one, and plain stores can move before volatile
stores (but not the other way, of course).  So only final fields get this
special treatment; if someone is unsafely publishing a ref and assumes the
volatile store in constructor is sufficient, they're mistaken :).

Thanks

Sent from my phone
On Nov 25, 2013 11:17 AM, "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>
wrote:

> Hi,
>
> I preprared a webrev with fixes for PPC for the VolatileIRIWTest of the
> torture test suite:
> http://cr.openjdk.java.net/~goetz/webrevs/8029101-0-raw/
>
> Example:
> volatile x=0, y=0
>   __________    __________    __________    __________
> | Thread 0 |  | Thread 1 |  | Thread 2 |  | Thread 3 |
>
>    write(x=1)    read(x)       write(y=1)    read(y)
>                  read(y)                     read(x)
>
> Disallowed:     x=1, y=0                    y=1, x=0
>
>
> Solution: This example requires multiple-copy-atomicity. This is only
> assured by the sync instruction and if it is executed in the threads
> doing the loads. Thus we implement volatile read as sync-load-acquire
> and omit the sync/MemBarVolatile after the volatile store.
> MemBarVolatile happens to be implemented by sync.
> We fix this in C2 and the cpp interpreter.
>
> This addresses a similar issue as fix "8012144: multiple SIGSEGVs
> fails on staxf" for taskqueue.hpp.
>
> Further this change contains a fix that assures that volatile fields
> written in constructors are visible before the reference gets
> published.
>
>
> Looking at the code, we found a MemBarRelease that to us, seems too strong.
> We think in parse1.cpp do_exits() a MemBarStoreStore should suffice.
> What do you think?
>
> Please review and test this change.
>
> Best regards,
>   Goetz.
>