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

[Vladimir Kozlov]

On 12/10/14 9:58 PM, Dean Long wrote:
> On 12/10/2014 8:24 PM, Vladimir Kozlov wrote:
>> On 12/10/14 12:44 PM, Andrew Haley wrote:
>>> On 12/10/2014 08:27 PM, David Holmes wrote:
>>>> If I had paid more attention to this earlier I would have suggested
>>>> reversing the sense of the UseBarriersForVolatile flag
>>>> (ElideBarriersForVolatiles?) because it makes it seem like using
>>>> barriers for volatiles is experimental - which of course it isn't.
>>>
>>> OK.
>>>
>>>> Also this seems C2 specific so shouldn't it be defined in c2_globals.hpp?
>>>
>>> Sure.  This is the Patch That Never Ends, after all.  :-)
>>>
>>> Let's see what the other reviewers say, and I'll wrap it all together.
>>>
>>> Thanks,
>>> Andrew.
>>>
>>
>> I agree with David about reversing flag and putting it into c2_globals.hpp. But, please, confirm that you left
>> volatile barriers generated by C1 compiler (in c1_LIR_Generator.cpp) as they are - I don't see any changes there.
>>
>> One thing left is barriers for volatile loads - there is inconsistency of code in library_call.cpp and parse3.cpp. We
>> discussed it already before.
>> If you want to remove MemBarAcquire for volatile loads then, please, also change code in Parse::do_get_xxx() in
>> parse3.cpp.
>
> Let me see if I'm on the same page.  Would that be something like:
>
>   272   // If reference is volatile, prevent following memory ops from
>   273   // floating up past the volatile read.  Also prevents commoning
>   274   // another volatile read.
>   275   if (field->is_volatile()) {
>   276     // Memory barrier includes bogus read of value to force load BEFORE membar
>   277     insert_mem_bar(ElideBarriersForVolatiles ? Op_MemBarCPUOrder : Op_MemBarAcquire, ld);
>   278   }
>
> ?  Does the "bogus read" comment and extra "ld" arg still apply if Op_MemBarCPUOrder is used?

It is there to be able remove membar if an object from which we load does not escape. See MemBarNode::Ideal().
So we can extend MemBarNode::Ideal() for MemBarCPUOrder case. Or don't add edge to 'ld' from the start - I think it is 
preferable.

Vladimir

>
> dl
>
>> If not, the code in library_call.cpp should avoid membars only for stores:
>>
>>      // For Stores, place a memory ordering barrier now.
>>      if (is_store) {
>> -      insert_mem_bar(Op_MemBarRelease);
>> +      insert_mem_bar(ElideBarriersForVolatiles ? Op_MemBarCPUOrder : Op_MemBarRelease);
>>      } else {
>>        if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
>>          insert_mem_bar(Op_MemBarVolatile);
>>        }
>>      }
>>
>>
>>    if (is_volatile) {
>>      if (!is_store) {
>>        insert_mem_bar(Op_MemBarAcquire);
>>      } else {
>>        if (!support_IRIW_for_not_multiple_copy_atomic_cpu) {
>> -        insert_mem_bar(Op_MemBarVolatile);
>> +        insert_mem_bar(ElideBarriersForVolatiles ? Op_MemBarCPUOrder : Op_MemBarVolatile);
>>        }
>>      }
>>    }
>>
>>
>> Thanks,
>> Vladimir
>>
>