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

[Dean Long]

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?

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
>