[aarch64-port-dev ] Don't use a release store when storing an OOP in a non-volatile field.

[Edward Nevill]

On Fri, 2014-08-22 at 16:58 +0100, Andrew Haley wrote:
> HotSpot always uses a release tore when writing OOP fields, because:
> 
>      // Non-volatile fields also need releasing stores if they hold an
>      // object reference, because the object reference might point to
>      // a freshly created object.
> 
> This isn't needed on AArch64 because we generate a memory barrier when
> the object is created and there is an address dependency from a read
> instruction to a program-order-later read or write instruction when
> the value read by the first is used to compute the address used for
> the second.  See 4.1, Address Dependencies, in
> http://www.cl.cam.ac.uk/~pes20/ppc-supplemental/test7.pdf for all the
> gory details.
> 
> I suspect that every CPU on which HotSpot runs respects address
> dependencies in the same way.

Hi,

I think it would be better to put the AARCH64 conditionalisation inside StoreNode::release_if_reference.

That way the conditionalisation on AARCH64 occurs in one place only which may may this easier to push upstream.

It also means that it catches the 1 additional place in parse2.cpp where I believe the store release can be removed.

void Parse::array_store(BasicType elem_type) {
  Node* adr = array_addressing(elem_type, 1);
  if (stopped())  return;     // guaranteed null or range check
  Node* val = pop();
  dec_sp(2);                  // Pop array and index
  const TypeAryPtr* adr_type = TypeAryPtr::get_array_body_type(elem_type);
  store_to_memory(control(), adr, val, elem_type, adr_type, StoreNode::release_if_reference(elem_type));
}

--- I believe this can also be removed -------------------------^^^^^^^^^^^^^

It does mean we have the slightly odd

StoreNode::release_if_reference(T_OBJECT)

But this would make more sense if the name was changed to 'release_if_necessary'.

Patch follows,
Regards,
Ed.

--- CUT HERE ---
diff -r b319f337ea31 src/share/vm/opto/memnode.hpp
--- a/src/share/vm/opto/memnode.hpp	Tue Aug 19 15:19:58 2014 +0100
+++ b/src/share/vm/opto/memnode.hpp	Fri Aug 22 19:08:32 2014 +0100
@@ -488,6 +488,12 @@
   // Conservatively release stores of object references in order to
   // ensure visibility of object initialization.
   static inline MemOrd release_if_reference(const BasicType t) {
+    // AArch64 doesn't need a release store because if there is an
+    // address dependency between a read and a write, then those
+    // memory accesses are observed in program order by all observers
+    // within the shareability domain.
+    AARCH64_ONLY(return unordered);
+
     const MemOrd mo = (t == T_ARRAY ||
                        t == T_ADDRESS || // Might be the address of an object reference (`boxing').
                        t == T_OBJECT) ? release : unordered;
diff -r b319f337ea31 src/share/vm/opto/parse2.cpp
--- a/src/share/vm/opto/parse2.cpp	Tue Aug 19 15:19:58 2014 +0100
+++ b/src/share/vm/opto/parse2.cpp	Fri Aug 22 19:08:32 2014 +0100
@@ -1744,7 +1744,7 @@
     a = pop();                  // the array itself
     const TypeOopPtr* elemtype  = _gvn.type(a)->is_aryptr()->elem()->make_oopptr();
     const TypeAryPtr* adr_type = TypeAryPtr::OOPS;
-    Node* store = store_oop_to_array(control(), a, d, adr_type, c, elemtype, T_OBJECT, MemNode::release);
+    Node* store = store_oop_to_array(control(), a, d, adr_type, c, elemtype, T_OBJECT, StoreNode::release_if_reference(T_OBJECT));
     break;
   }
   case Bytecodes::_lastore: {
--- CUT HERE ---