MemNode::unordered/release/acquire use in relation to volatile stores and loads

[Volker Simonis]

On Mon, Aug 4, 2014 at 11:13 PM, Vladimir Kozlov
<vladimir.kozlov at oracle.com> wrote:
> I would say that you are correct (MemNode::acquire is needed for volatile
> load).
> Lets hear Goetz answer.
>

Hi David, Vladimir,

unfortunately, Goetz is on a sabbatical leave until mid September...

> Thanks,
> Vladimir
>
>
> On 8/3/14 4:51 PM, David Holmes wrote:
>>
>> In ./share/vm/opto/library_call.cpp
>>
>> Why is it that for volatile stores we have special checks for which
>> MemNode to use:
>>
>>    MemNode::MemOrd mo = is_volatile ? MemNode::release :
>> MemNode::unordered;
>>      if (type != T_OBJECT ) {
>>        (void) store_to_memory(control(), adr, val, type, adr_type, mo,
>> is_volatile);
>>      } else {
>>
>> but for loads it is always unordered eg:
>>
>>    Node* p = make_load(control(), adr, value_type, type, adr_type,
>> MemNode::unordered, is_volatile);
>>

That seems indeed strange. Especially because in our internal SAP JVM
version we have:

Node* p = make_load(control(), adr, value_type, type, adr_type,
is_volatile ? MemNode::acquire : MemNode::unordered, is_volatile);

I've cc-ed Martin who did this change in our sources (unfortunately
there's still a little diff between them and the OpenJDK :(
He will be back next week and maybe he has some more insights?

>> ?? I would have expected MemNode::acquire as in opto/parse3.cpp
>>
>>    // Build the load.
>>    //
>>    MemNode::MemOrd mo = is_vol ? MemNode::acquire : MemNode::unordered;
>>    bool needs_atomic_access = is_vol || AlwaysAtomicAccesses;
>>    Node* ld = make_load(NULL, adr, type, bt, adr_type, mo,
>> needs_atomic_access);
>>

And there's also another instance of this pattern in the method
LibraryCallKit::load_field_from_object() in library_call.cpp which
should be probably fixed as well:

// Build the load.
Node* loadedField = make_load(NULL, adr, type, bt, adr_type,
MemNode::unordered, is_vol);

Regards,
Volker

>>
>>
>> Thanks,
>> David
>>
>