Replace MemBarRelease for final field write with MemBarStoreStore

[Andrew Haley]

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.