[aarch64-port-dev ] AARCH64: 8064611: Changes to HotSpot shared code

[Andrew Haley]

On 11/20/2014 06:13 PM, Andrew Haley wrote:
> On 11/20/2014 06:05 PM, Vladimir Kozlov wrote:
>> I based the name on your comment:
>>
>> +  // AArch64 uses store release (which does everything we need to keep
>> +  // the machine in order) but we still need a compiler barrier here.
> 
> Ah.  Okay, I'll have to think of a good name for it, then.
> 
>> You can name it as you like. Our main suggestion is to use such Boolean 
>> constant and normal if() statements instead of ifdef AARCH64 and 
>> AARCH64_ONLY/NOT_AARCH64 macros in C2 code (src/share/vm/opto/* files).
>>
>> We already do similar things for PPC64 port which sets 
>> support_IRIW_for_* constant.
> 
> Okay,

I've done something similar but more useful.  I've added an
experimental flag: UseBarriersForVolatile.  This defaults to true for
all targets, but we can override it in the back end.  That gives me
the chance to do some benchmarking on various AArch64 targets to see
which ones benefit from the new load acquire/store release
instructions.

I have kept AARCH64_ONLY for two hunks:

--- old/src/share/vm/opto/memnode.hpp	2014-11-21 12:09:22.766963837 -0500
+++ new/src/share/vm/opto/memnode.hpp	2014-11-21 12:09:22.546983320 -0500
@@ -503,6 +503,10 @@
   // 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 here because object
+    // initialization contains the necessary barriers.
+    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;

--- old/src/share/vm/opto/graphKit.cpp	2014-11-21 12:09:20.017207376 -0500
+++ new/src/share/vm/opto/graphKit.cpp	2014-11-21 12:09:19.787227745 -0500
@@ -3813,7 +3813,8 @@

   // Smash zero into card
   if( !UseConcMarkSweepGC ) {
-    __ store(__ ctrl(), card_adr, zero, bt, adr_type, MemNode::release);
+    __ store(__ ctrl(), card_adr, zero, bt, adr_type,
+             NOT_AARCH64(MemNode::release) AARCH64_ONLY(MemNode::unordered));
   } else {
     // Specialized path for CM store barrier
     __ storeCM(__ ctrl(), card_adr, zero, oop_store, adr_idx, bt, adr_type);

The first hunk is only required by IA64 as far as I am aware, but I
am nervous about making it IA64_ONLY.  The second hunk is a release
node which is not as far as I am aware required by any target, and
should simply be removed.

This isn't a RFA because it's not tested yet, but what do you think?

Andrew.