[aarch64-port-dev ] RFR: 8144993: Elide redundant memory barrier after AllocationNode

[Hui Shi]

Thanks Gotez!

case 1) can be handle with current patch. BCEA information is getting from
owning method when inserting release memory barrier for final field write.
Final field is initialized in its owning allocation node's constructor
method.

Following code is in parse::do_exits, alloc->compute_MemBar_redundancy  get
constructor method's BCEA information and check if allocation escape in
constructor method.

   if (method()->is_initializer() &&
         (wrote_final() ||
            PPC64_ONLY(wrote_volatile() ||)
            (AlwaysSafeConstructors && wrote_fields()))) {
     _exits.insert_mem_bar(Op_MemBarRelease, alloc_with_final());
+
+    // If Memory barrier is created for final fields write
+    // and allocation node does not escape the initialize method,
+    // then barrier introduced by allocation node can be removed.
+    if (DoEscapeAnalysis && alloc_with_final()) {
+      AllocateNode *alloc =
AllocateNode::Ideal_allocation(alloc_with_final(), &_gvn);
+      alloc->compute_MemBar_redundancy(method());
+    }


Regards
Hui


On 18 December 2015 at 18:43, Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
wrote:

> Hi Hui,
>
> > Subject: Re: RFR: 8144993: Elide redundant memory barrier after
> > AllocationNode
> >
> > Thanks Andrew, Goetz and all!
> >
> > Major concern is will removing storestore barrier cause other threads
> read
> > stale data for newly allocated object. Other threads include java thread
> or
> > concurrent GC thread. It should be safe with following analysis.
> >
> > 1. If BCEA result "this"(b) escapes in its initializer, change will not
> optimize
> > storestore barrier.
> > 2. If BCEA result "this"(b) does not escape in its initializer, it's
> safe to remove
> > storestore.
> >    2.1 If there is a safe point between storestore and release, b is
> visible to GC
> > in initializer, but at safe point, it should have a memory barrier.
> >    2.2 If there is no safe point between storestore and release. b will
> be visible
> > to other thread after release memory barrier.
> I think this describes the situation correctly wrt. to my counterexample.
> I'm
> not sure whether there are other possibilities.
>
> Is the test for 1.) already implemented?
> How do you do this?  Is inlining of the constructor delayed when you do
> your optimization, so you can find the call to it?  Or do you find the
> BCEA information
> via the class that is reachable over the type information?  How do you
> known then
> which constructor was called if there are several ones?
>
> Best regards,
>   Goetz.
>
>
>
>
>
> >
> > Case #1
> > A a = new A();
> > safepoint // a can be reached from GC
> > new B(a)
> >
> > allocation
> > -------
> > b.klass =...
> > b.markword =...
> > b.f1 = 0
> > ..
> > b.fn = 0
> > storestore
> > -------- init start
> > ....
> > a.x = this;  // b might visible to other threads here
> > ....
> > release
> > -------- init end
> >
> > BCEA result indicate "this"(b) is not local and not arg_stack. So "b"
> will be
> > treated as escaped in its initialzer, so change will not optimize
> storestore
> > barrier.
> > [EA] estimated escape information for B::<init>
> >      non-escaping args:      {}
> >      stack-allocatable args: {1}
> >      return non-local value
> >      modified args:     0x6    0x6
> >      flags:
> > b="this"  is not local and not arg_stack
> > a        is arg_stack means it is passed in and not assigned to other
> object in
> > initializer.
> >
> > Case #2.1
> > allocation
> > -------
> > b.klass =...
> > b.markword =...
> > b.f1 = 0
> > ..
> > b.fn = 0
> > storestore
> > -------- init start
> > ....
> > safepoint  // "this" is in oop map and might visible to GC thread here
> > ....
> > release
> > -------- init end
> >
> > Case #2.2
> > allocation
> > -------
> > b.klass =...
> > b.markword =...
> > b.f1 = 0
> > ..
> > b.fn = 0
> > storestore
> > -------- init start
> > ....
> > release
> > -------- init end
> >
> > Regards
> > Hui
> >
> > On 16 December 2015 at 00:15, Andrew Haley <aph at redhat.com
> > <mailto:aph at redhat.com> > wrote:
> >
> >
> >       On 12/15/2015 04:01 PM, Lindenmaier, Goetz wrote:
> >
> >       > Further, if the object is NoEscape it might not be scalar
> >       > replaced. If I remember correctly, there are various conditions,
> >       > e.g., too big, allocated in loop.
> >
> >       Well, that's the killer.  The definition of "escape" we need to use
> >       here is the really, truly, honest-to-goodness one: that this object
> >       never becomes visible to any other thread by any means.  Unless
> > that
> >       is so, all bets are off.  In this case, what is intended is
> "appears
> >       in an OOP map".
> >
> >       Andrew.
> >
> >
>
>