RFR(M): 8141635: Implement new Unsafe intrinsics on POWER

[Aleksey Shipilev]

Hi Martin,

On 05/03/2016 01:16 PM, Doerr, Martin wrote:
> According to JEP 193, Compare and Swap/Exchange methods need to behave
> like volatile load + volatile store.
> 
> Therefore, I had to make a change to the shared library_call
> implementation (support_IRIW_for_not_multiple_copy_atomic_cpu was not
> yet included).

Yes, CAS/CAE are supposed to be sequentially consistent. This change
looks correct, if a hardware CAS is not SC:

2852   switch (access_kind) {
...
2856     case Release:
2857       insert_mem_bar(Op_MemBarRelease);
2858       break;
2859     case Volatile:
2860       if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
2861         insert_mem_bar(Op_MemBarVolatile);
2862       } else {
2863         insert_mem_bar(Op_MemBarRelease);
2864       }
2865       break;

Actually, that seems to mean current CAS implementation on POWER, prior
to VarHandles, was also broken.

> I also assert that GetAndAdd and GetAndSet are only used with seq_cst
> semantics, because the memory order is not passed to these intrinsics.

Yes, good.

> C++2011 supports specifying weaker semantics for failed cases of
> atomic_compare_exchange than for successful ones.
> 
> As I understand JEP 193, Java expects the semantics for failed cases to
> be the same as for successful cases.
> 
> That means the memory barrier instruction must be executed even though
> the store was not executed. So the failed Compare and Swap/Exchange
> behaves like a volatile load.

Yes, that's the difference between VarHandles and C++11 semantics.
VarHandles provide a single memory ordering, regardless of success/failure.

I wonder, however, why you need sync() after CAS? C++11 mappings on
POWER: https://www.cl.cam.ac.uk/~pes20/cpp/cpp0xmappings.html ...say
that Cmpxchg SeqCst is:

       hwsync;     // Op_MemBarVolatile
_loop: lwarx;
       cmp;
       bc _exit;
       stwcx.;
       bc _loop;
       isync;
_exit:

Load Seq Cst, which is similar to failing CAS semantics, is:

       hwsync;
       ld;
       cmp;
       bc;
       isync

Therefore isync() is enough after CAS in ppc.ad? Also, why not fix the
CAS in MacroAssembler with these barriers to match C++11 mappings,
instead of doing special cases in .ad?

> They do not return the loaded value, but only report whether the Swap
> was performed or not. The failure cause is not clearly identifiable and
> I don’t see any purpose of executing a memory barrier in case of failure.

I don't think this is about the returned value, but rather about the
transitive memory effects. So, I would post the barrier on the exit path
for weakCAS{Acquire,Release} too.

Thanks,
-Aleksey

P.S. Note that we have weakCompareAndSetVolatile intrinsics in works,
which would probably require a few additional matches in .ad...
  https://bugs.openjdk.java.net/browse/JDK-8155965

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160503/433553bb/signature-0001.asc>