[concurrency-interest] RFR: 8065804: JEP 171: Clarifications/corrections for fence intrinsics

[Martin Buchholz]

On Thu, Dec 4, 2014 at 5:55 PM, David Holmes <david.holmes at oracle.com> wrote:

> In general phrasing like: " also known as a LoadLoad plus LoadStore barrier
> ..." is misleading to me as these are not "aliases"- the loadFence (in this
> case) is being specified to have the same semantics as the
> loadload|storeload. It should say "corresponds to a LoadLoad plus LoadStore
> barrier"

+     * Ensures that loads before the fence will not be reordered with loads and
+     * stores after the fence; also known as a LoadLoad plus LoadStore barrier,

I don't understand this.  I believe they _are_ aliases.  The first
clause perfectly describes a "LoadLoad plus LoadStore barrier".

 - as per the "Corresponds to a C11 ...". And referring to things
> like "load-acquire fence" is meaningless without some reference to a
> definition - who defines a load-acquire fence? Is there a universal
> definition? I would be okay with something looser eg:

Well, I'm defining "load-acquire fence" here in the javadoc - I'm
claiming that loadFence is also known via other terminology, including
"load-acquire fence".  Although it's true that "load-acquire fence" is
also used to refer to the corresponding C11 fence, which has subtly
different semantics.

> /**
>   * Ensures that loads before the fence will not be reordered with loads and
>   * stores after the fence. Corresponds to a LoadLoad plus LoadStore
> barrier,
>   * and also to the C11 atomic_thread_fence(memory_order_acquire).
>   * Sometimes referred to as a "load-acquire fence".
>   *
>
> Also I find this comment strange:
>
> !      * A pure StoreStore fence is not provided, since the addition of
> LoadStore
> !      * is almost always desired, and most current hardware instructions
> that
> !      * provide a StoreStore barrier also provide a LoadStore barrier for
> free.
>
> because inside hotspot we use storeStore barriers a lot, without any
> loadStore at the same point.

I believe the use of e.g. OrderAccess::storestore in the hotspot
sources is unfortunate.

The actual implementations of storestore (see below) seem to
universally give you the stronger ::release barrier, and it seems
likely that hotspot engineers are implicitly relying on that, that
some uses of ::storestore in the hotspot sources are bugs (should be
::release instead) and that there is very little potential performance
benefit from using ::storestore instead of ::release, precisely
because the additional loadstore barrier is very close to free on all
current hardware.  Writing correct code using ::storestore is harder
than ::release, which is already difficult enough. C11 doesn't provide
a corresponding fence, which is a strong hint.


./bsd_zero/vm/orderAccess_bsd_zero.inline.hpp:71:inline void
OrderAccess::storestore() { release(); }
./linux_sparc/vm/orderAccess_linux_sparc.inline.hpp:35:inline void
OrderAccess::storestore() { release(); }
./aix_ppc/vm/orderAccess_aix_ppc.inline.hpp:73:inline void
OrderAccess::storestore() { inlasm_lwsync();  }
./linux_zero/vm/orderAccess_linux_zero.inline.hpp:70:inline void
OrderAccess::storestore() { release(); }
./solaris_sparc/vm/orderAccess_solaris_sparc.inline.hpp:40:inline void
OrderAccess::storestore() { release(); }
./linux_ppc/vm/orderAccess_linux_ppc.inline.hpp:75:inline void
OrderAccess::storestore() { inlasm_lwsync();  }
./solaris_x86/vm/orderAccess_solaris_x86.inline.hpp:40:inline void
OrderAccess::storestore() { release(); }
./linux_x86/vm/orderAccess_linux_x86.inline.hpp:35:inline void
OrderAccess::storestore() { release(); }
./bsd_x86/vm/orderAccess_bsd_x86.inline.hpp:35:inline void
OrderAccess::storestore() { release(); }
./windows_x86/vm/orderAccess_windows_x86.inline.hpp:35:inline void
OrderAccess::storestore() { release(); }