Release store in C2 putfield

[Volker Simonis]

I've just checked that we only use this flag (i.e.
as_Store()->is_unordered()/as_Store()->is_release()) on our Itanium
port. Other platforms use the MemBarRelease/MemBarStoreStore nodes
which are there anyway. On Itanium, we don't use these nodes that's
why we need the flag.

But I don't see how this bothers anybody. If you don't use this flag
in your .ad file in your Store nodes it will have no side effect. Or
am I missing something?

Regards,
Martin and Volker


On Thu, Sep 4, 2014 at 2:28 AM, Vladimir Kozlov
<vladimir.kozlov at oracle.com> wrote:
> In general I am fine with such platform specific change in shared code.
>
> But in current case non-volatile release_store code affects only PPC64. As
> Aleksey suggested may be we should use NOT_PPC64() macro instead.
> Also I don't see calls as_Store()->is_unordered() in ppc.ad file. Looks like
> PPC64 doesn't check this flag.
>
> Thanks,
> Vladimir
>
>
> On 9/3/14 11:16 AM, Andrew Haley wrote:
>>
>> On 09/03/2014 06:21 PM, Vladimir Kozlov wrote:
>>>
>>> Andrew,
>>>
>>> Do you need unordered in Parse::array_store() too?
>>
>>
>> Yes.
>>
>>> Another way of doing it is to define MemNode::release_if_reference() in
>>> .ad files in 'source %{' section.
>>
>>
>> Yes, that's what we've got now.  Thanks.
>>
>> Andrew.
>>
>>
>> # HG changeset patch
>> # User Edward Nevill edward.nevill at linaro.org
>> # Date 1409307165 -3600
>> #      Fri Aug 29 11:12:45 2014 +0100
>> # Node ID fc245bc14fa3589074c78ceb0e25ecf36ee3e110
>> # Parent  32fae3443576ac6b4b5ac0770c0829ce6c08764e
>> Dont use a release store when storing an OOP in a non-volatile field.
>>
>> diff -r 32fae3443576 -r fc245bc14fa3 src/share/vm/opto/memnode.hpp
>> --- a/src/share/vm/opto/memnode.hpp     Mon Sep 01 13:10:18 2014 -0400
>> +++ b/src/share/vm/opto/memnode.hpp     Fri Aug 29 11:12:45 2014 +0100
>> @@ -503,6 +503,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 32fae3443576 -r fc245bc14fa3 src/share/vm/opto/parse2.cpp
>> --- a/src/share/vm/opto/parse2.cpp      Mon Sep 01 13:10:18 2014 -0400
>> +++ b/src/share/vm/opto/parse2.cpp      Fri Aug 29 11:12:45 2014 +0100
>> @@ -1689,7 +1689,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: {
>>
>