[aarch64-port-dev ] Replace MemBarRelease for final field write with MemBarStoreStore

[Vitaly Davidovich]

>
> Probably not.  See http://www.hboehm.info/c++mm/no_write_fences.html
> for an explanation.


Is that really relevant to final/stable field writes though?

On Wed, Sep 2, 2015 at 9:41 AM, Andrew Haley <aph at redhat.com> wrote:

> On 09/02/2015 02:25 PM, Hui Shi wrote:
>
> > Could anyone help review and comments this change?  This change
> > involves memory barrier, escape analysis and maybe PPC.
> >
> > In C2 compiler, MemBarRelease node is generated at end of method if
> > it writes final/stable field. Is it better to use MemBarStoreStore
> > here?
>
> Probably not.  See http://www.hboehm.info/c++mm/no_write_fences.html
> for an explanation.
>
> >  Reasons are:
>
> > 1. MemBarRelease represents loadstore + storestore barrier, it costs
> > more than MemBarStoreStore on RMO(Relax Memory Order) platforms like
> > aarch64.
> >     For attached small case TestWriteFinal_MemoryBarrier.java, with
> > storestore memory barrier it is 40% faster on aarch64.
>
> On at least one AArch64 implementation, you mean?
>
> > 2. According to JSR133 cook book, it only requires storestore
> > barrier. Are there some corner cases needs loadstore barrier?
> > 3. In C1 implementation (GraphBuilder::method_return), it only
> > inserts storestore barrier (lir_membar_storestore), barrier for
> > stable field in C1 is missing, maybe we need add memory barrier in
> > C1 too for stable field?
>
> Probably, but C1 is much more conservative with its optimizations.
>
> > Browsing hotspot repository history, is this an early conservative
> > handling which not updated yet?
> > 1. MemBarRelease is used for final field store since first hotspot
> > version. At that time, MemBarStoreStore is not defined yet.
> > 2 .MemBarStoreStore is added later for barrier after object
> > allocation and array copy
> > (http://hg.openjdk.java.net/aarch64-port/jdk8/hotspot/rev/1dc233a8c7fe)
>
> StoreStore is fine for object allocation; any other field writes
> should probably use a release.  But this part of C2 is extremely
> delicate, and various parts of it "know" in what circumstances
> StoreStore is used, and know that those circumstances are very
> limited.
>
> > This is not true for MemBar node after final fields store, whose
> > MemBarNode::Precedent input is initialized with
> > _alloc_with_final. Checking Parse::do_put_xxx
>
> > 1. _alloc_with_final might not be set. For example object type is
> > not boxed type.
> > 2. _alloc_with_final is set with store obj node, this usually is
> > CheckCastPP instead of
> > allocation->proj_out(AllocateNode::RawAddress). if not align with
> > MemBarStoreStore 's assumption, this will trigger assertion in above
> > ConnectionGraph::optimize_ideal_graph code.
> >
> > In attached patch, changes include:
>
> I'd really like you to attach your patches more firmly or use a web
> site!
>
> But really, I'm not going to accept a weakening of the barrier because of
> "Why ordering constraints are never limited to loads or stores"
>
> Andrew.
>