RFR(M): 8154736: enhancement of cmpxchg and copy_to_survivor for ppc64

[Kim Barrett]

> On Oct 5, 2016, at 9:36 PM, David Holmes <david.holmes at oracle.com> wrote:
> 
> On 5/10/2016 10:36 AM, Hiroshi H Horii wrote:
>> Dear David,
>> 
>> Thank you for your comments.
>> 
>> I just used to think that it may be better that copy_to_survivor_space
>> doesn't return forwardee if CAS was failed in order to prevent from
>> reading fields in forwardee. But as you pointed, this extends fix for
>> this topic.
>> 
>> I removed two NULL assignments from the previous wevrev.
>> http://cr.openjdk.java.net/~horii/8154736/webrev.03/
> 
> Which simply takes us back to where we were. It may not be safe for the caller of those methods to access the fields of the returned "forwardee".
> 
> Sorry but I'm not seeing anything here that justifies removing the barriers from the cas in this code. GC lurkers feel free to jump in here - this is your code afterall! ;-)
> 
> David
> -----

Using a CAS with memory_order_relaxed in copy_to_survivor_space seems
to me to be extremely fragile and hard to reason about.  The places
where that copied object might escape to and be examined seem to be
myriad.  And not only do we need to worry about them today, but also
for future maintenance.  Even if it can modified and shown to be
correct today, it would be very easy to intoduce a bug later, as
should be obvious from the various issues pointed out so far during
this review.

The key issue here is that we copy obj into new_obj, and then make
new_obj accessible to other threads via the CAS.  Those other threads
might attempt to access data in new_obj.  This suggests the CAS ought
to have at least a release fence to ensure the copy is complete before
the CAS is performed.  No amount of fencing on the read side (such as
in the work stealing) can remove that need.

And that might be all that is needed.  On the post-CAS side, we load
the forwardee and then load values from it.  I thik we can use
implicit consume with dependent loads (except on Alpha) plus the
suggested release fence to get the desired effect.  (If not, use an
acquire form of forwardee()?)

I'm not certain that just a release fence is sufficient (I'm less
familiar with ParallelGC than I'd like for looking at something like
this), but I'm pretty sure I wouldn't want to go any weaker than that.