# HG changeset patch
# User mdoerr
# Date 1567599714 -7200
# Node ID dee322336e1727b82203bfafca812e9be17460f7
# Parent  a645b2f7bece55a58227ff825dec6c87bd1dc8a6
8230434: [C1, C2] Release barrier for volatile field stores in constructors implemented inconsistently
Reviewed-by: shade, lucy

diff -r a645b2f7bece -r dee322336e17 src/hotspot/share/c1/c1_GraphBuilder.cpp
--- a/src/hotspot/share/c1/c1_GraphBuilder.cpp	Wed Sep 04 09:47:55 2019 +0200
+++ b/src/hotspot/share/c1/c1_GraphBuilder.cpp	Wed Sep 04 14:21:54 2019 +0200
@@ -1467,11 +1467,12 @@
     call_register_finalizer();
   }
 
+  // The conditions for a memory barrier are described in Parse::do_exits().
   bool need_mem_bar = false;
   if (method()->name() == ciSymbol::object_initializer_name() &&
-      (scope()->wrote_final() || (AlwaysSafeConstructors && scope()->wrote_fields())
-                              || (support_IRIW_for_not_multiple_copy_atomic_cpu && scope()->wrote_volatile())
-     )){
+       (scope()->wrote_final() ||
+         (AlwaysSafeConstructors && scope()->wrote_fields()) ||
+         (support_IRIW_for_not_multiple_copy_atomic_cpu && scope()->wrote_volatile()))) {
     need_mem_bar = true;
   }
 
diff -r a645b2f7bece -r dee322336e17 src/hotspot/share/opto/parse1.cpp
--- a/src/hotspot/share/opto/parse1.cpp	Wed Sep 04 09:47:55 2019 +0200
+++ b/src/hotspot/share/opto/parse1.cpp	Wed Sep 04 14:21:54 2019 +0200
@@ -979,15 +979,18 @@
   //    Rather than put a barrier on only those writes which are required
   //    to complete, we force all writes to complete.
   //
-  // 2. On PPC64, also add MemBarRelease for constructors which write
-  //    volatile fields. As support_IRIW_for_not_multiple_copy_atomic_cpu
-  //    is set on PPC64, no sync instruction is issued after volatile
-  //    stores. We want to guarantee the same behavior as on platforms
-  //    with total store order, although this is not required by the Java
-  //    memory model. So as with finals, we add a barrier here.
+  // 2. Experimental VM option is used to force the barrier if any field
+  //    was written out in the constructor.
   //
-  // 3. Experimental VM option is used to force the barrier if any field
-  //    was written out in the constructor.
+  // 3. On processors which are not CPU_MULTI_COPY_ATOMIC (e.g. PPC64),
+  //    support_IRIW_for_not_multiple_copy_atomic_cpu selects that
+  //    MemBarVolatile is used before volatile load instead of after volatile
+  //    store, so there's no barrier after the store.
+  //    We want to guarantee the same behavior as on platforms with total store
+  //    order, although this is not required by the Java memory model.
+  //    In this case, we want to enforce visibility of volatile field
+  //    initializations which are performed in constructors.
+  //    So as with finals, we add a barrier here.
   //
   // "All bets are off" unless the first publication occurs after a
   // normal return from the constructor.  We do not attempt to detect
@@ -995,9 +998,9 @@
   // exceptional returns, since they cannot publish normally.
   //
   if (method()->is_initializer() &&
-        (wrote_final() ||
-           PPC64_ONLY(wrote_volatile() ||)
-           (AlwaysSafeConstructors && wrote_fields()))) {
+       (wrote_final() ||
+         (AlwaysSafeConstructors && wrote_fields()) ||
+         (support_IRIW_for_not_multiple_copy_atomic_cpu && wrote_volatile()))) {
     _exits.insert_mem_bar(Op_MemBarRelease, alloc_with_final());
 
     // If Memory barrier is created for final fields write