AARCH64: 8064611: Changes to HotSpot shared code

[Vladimir Kozlov]

On 11/21/14 9:33 AM, Andrew Haley wrote:
> 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.

Okay.

>
> 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;

This could be needed for ppc64 too, not only for IA64.

>
> --- 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);

Looks like PPC64 needs that. In ppc.ad:

   // Use release_store for card-marking to ensure that previous
   // oop-stores are visible before the card-mark change.
   enc_class enc_cms_card_mark(memory mem, iRegLdst releaseFieldAddr) %{

>
> 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?

Since it affects ppc64 and ia64 we need to ask Goetz and Co.
I would suggest to put both these places under platform specific 
flags/bool constant.

Thanks,
Vladimir

>
> Andrew.
>