Variant 3: Handle everything, including reads by compilers

Author: shipilev

src/hotspot/share/c1/c1_GraphBuilder.cpp

@@ -1563,7 +1563,7 @@ void GraphBuilder::method_return(Value x, bool ignore_return) {
   // The conditions for a memory barrier are described in Parse::do_exits().
   bool need_mem_bar = false;
   if (method()->name() == ciSymbols::object_initializer_name() &&
-       (scope()->wrote_final() ||
+       (scope()->wrote_final() || scope()->wrote_stable() ||
          (AlwaysSafeConstructors && scope()->wrote_fields()) ||
          (support_IRIW_for_not_multiple_copy_atomic_cpu && scope()->wrote_volatile()))) {
     need_mem_bar = true;
@@ -1741,15 +1741,17 @@ void GraphBuilder::access_field(Bytecodes::Code code) {
     }
   }
 
-  if (field->is_final() && (code == Bytecodes::_putfield)) {
-    scope()->set_wrote_final();
-  }
-
   if (code == Bytecodes::_putfield) {
     scope()->set_wrote_fields();
     if (field->is_volatile()) {
       scope()->set_wrote_volatile();
     }
+    if (field->is_final()) {
+      scope()->set_wrote_final();
+    }
+    if (field->is_stable()) {
+      scope()->set_wrote_stable();
+    }
   }
 
   const int offset = !needs_patching ? field->offset_in_bytes() : -1;

src/hotspot/share/c1/c1_IR.cpp

@@ -146,6 +146,7 @@ IRScope::IRScope(Compilation* compilation, IRScope* caller, int caller_bci, ciMe
   _wrote_final        = false;
   _wrote_fields       = false;
   _wrote_volatile     = false;
+  _wrote_stable       = false;
   _start              = nullptr;
 
   if (osr_bci != -1) {

src/hotspot/share/c1/c1_IR.hpp

@@ -149,6 +149,7 @@ class IRScope: public CompilationResourceObj {
   bool          _wrote_final;                    // has written final field
   bool          _wrote_fields;                   // has written fields
   bool          _wrote_volatile;                 // has written volatile field
+  bool          _wrote_stable;                   // has written @Stable field
   BlockBegin*   _start;                          // the start block, successsors are method entries
 
   ResourceBitMap _requires_phi_function;         // bit is set if phi functions at loop headers are necessary for a local variable
@@ -187,6 +188,8 @@ class IRScope: public CompilationResourceObj {
   bool          wrote_fields    () const         { return _wrote_fields; }
   void          set_wrote_volatile()             { _wrote_volatile = true; }
   bool          wrote_volatile    () const       { return _wrote_volatile; }
+  void          set_wrote_stable()               { _wrote_stable = true; }
+  bool          wrote_stable() const             { return _wrote_stable; }
 };
 
 

src/hotspot/share/c1/c1_LIRGenerator.cpp

@@ -1677,7 +1677,18 @@ void LIRGenerator::do_StoreField(StoreField* x) {
 
   DecoratorSet decorators = IN_HEAP;
   if (is_volatile) {
+    // Volatile access, full SC.
     decorators |= MO_SEQ_CST;
+  } else if (x->field()->is_stable() && !x->field()->is_final() &&
+             is_reference_type(field_type)) {
+    // For reference @Stable fields, make sure we publish the contents
+    // safely. We need this to make sure compilers see a proper value when
+    // constant folding the access. Final @Stable fields are already
+    // handled in constructors.
+    decorators |= MO_RELEASE;
+  } else {
+    // Everything else is unordered.
+    decorators |= MO_UNORDERED;
   }
   if (needs_patching) {
     decorators |= C1_NEEDS_PATCHING;

src/hotspot/share/ci/ciInstance.cpp

@@ -78,7 +78,11 @@ ciConstant ciInstance::field_value_impl(BasicType field_btype, int offset) {
     case T_LONG:    value = ciConstant(obj->long_field(offset)); break;
     case T_OBJECT:  // fall through
     case T_ARRAY: {
-      oop o = obj->obj_field(offset);
+      // In case we are reading the constant off a reference @Stable field,
+      // we need to match the releasing store with this acquire. There is no easy
+      // way to know this from the offset, but it should not hurt to ask this for
+      // all reference fields.
+      oop o = obj->obj_field_acquire(offset);
 
       // A field will be "constant" if it is known always to be
       // a non-null reference to an instance of a particular class,

src/hotspot/share/classfile/classFileParser.cpp

@@ -1970,7 +1970,7 @@ AnnotationCollector::annotation_index(const ClassLoaderData* loader_data,
     }
     case VM_SYMBOL_ENUM_NAME(jdk_internal_vm_annotation_Stable_signature): {
       if (_location != _in_field)   break;  // only allow for fields
-      if (!privileged)              break;  // only allow in privileged code
+      if (RestrictStable && !privileged) break;  // only allow in privileged code
       return _field_Stable;
     }
     case VM_SYMBOL_ENUM_NAME(jdk_internal_vm_annotation_Contended_signature): {

src/hotspot/share/opto/parse.hpp

@@ -358,7 +358,7 @@ class Parse : public GraphKit {
   bool          _wrote_volatile;     // Did we write a volatile field?
   bool          _wrote_stable;       // Did we write a @Stable field?
   bool          _wrote_fields;       // Did we write any field?
-  Node*         _alloc_with_final;   // An allocation node with final field
+  Node*         _alloc_with_final_or_stable; // An allocation node with final or @Stable field
 
   // Variables which track Java semantics during bytecode parsing:
 
@@ -403,10 +403,10 @@ class Parse : public GraphKit {
   void      set_wrote_stable(bool z)  { _wrote_stable = z; }
   bool         wrote_fields() const   { return _wrote_fields; }
   void     set_wrote_fields(bool z)   { _wrote_fields = z; }
-  Node*    alloc_with_final() const   { return _alloc_with_final; }
-  void set_alloc_with_final(Node* n)  {
-    assert((_alloc_with_final == nullptr) || (_alloc_with_final == n), "different init objects?");
-    _alloc_with_final = n;
+  Node*    alloc_with_final_or_stable() const   { return _alloc_with_final_or_stable; }
+  void set_alloc_with_final_or_stable(Node* n)  {
+    assert((_alloc_with_final_or_stable == nullptr) || (_alloc_with_final_or_stable == n), "different init objects?");
+    _alloc_with_final_or_stable = n;
   }
 
   Block*             block()    const { return _block; }

src/hotspot/share/opto/parse1.cpp

@@ -412,7 +412,7 @@ Parse::Parse(JVMState* caller, ciMethod* parse_method, float expected_uses)
   _wrote_volatile = false;
   _wrote_stable = false;
   _wrote_fields = false;
-  _alloc_with_final = nullptr;
+  _alloc_with_final_or_stable = nullptr;
   _block = nullptr;
   _first_return = true;
   _replaced_nodes_for_exceptions = false;
@@ -984,8 +984,8 @@ void Parse::do_exits() {
   // Figure out if we need to emit the trailing barrier. The barrier is only
   // needed in the constructors, and only in three cases:
   //
-  // 1. The constructor wrote a final. The effects of all initializations
-  //    must be committed to memory before any code after the constructor
+  // 1. The constructor wrote a final or a @Stable field. All these
+  //    initializations must be ordered before any code after the constructor
   //    publishes the reference to the newly constructed object. Rather
   //    than wait for the publication, we simply block the writes here.
   //    Rather than put a barrier on only those writes which are required
@@ -1010,34 +1010,23 @@ void Parse::do_exits() {
   // exceptional returns, since they cannot publish normally.
   //
   if (method()->is_initializer() &&
-       (wrote_final() ||
+       (wrote_final() || wrote_stable() ||
          (AlwaysSafeConstructors && wrote_fields()) ||
          (support_IRIW_for_not_multiple_copy_atomic_cpu && wrote_volatile()))) {
+    Node* recorded_alloc = alloc_with_final_or_stable();
     _exits.insert_mem_bar(UseStoreStoreForCtor ? Op_MemBarStoreStore : Op_MemBarRelease,
-                          alloc_with_final());
+                          recorded_alloc);
 
     // If Memory barrier is created for final fields write
     // and allocation node does not escape the initialize method,
     // then barrier introduced by allocation node can be removed.
-    if (DoEscapeAnalysis && alloc_with_final()) {
-      AllocateNode* alloc = AllocateNode::Ideal_allocation(alloc_with_final());
+    if (DoEscapeAnalysis && (recorded_alloc != nullptr)) {
+      AllocateNode* alloc = AllocateNode::Ideal_allocation(recorded_alloc);
       alloc->compute_MemBar_redundancy(method());
     }
     if (PrintOpto && (Verbose || WizardMode)) {
       method()->print_name();
-      tty->print_cr(" writes finals and needs a memory barrier");
-    }
-  }
-
-  // Any method can write a @Stable field; insert memory barriers
-  // after those also. Can't bind predecessor allocation node (if any)
-  // with barrier because allocation doesn't always dominate
-  // MemBarRelease.
-  if (wrote_stable()) {
-    _exits.insert_mem_bar(Op_MemBarRelease);
-    if (PrintOpto && (Verbose || WizardMode)) {
-      method()->print_name();
-      tty->print_cr(" writes @Stable and needs a memory barrier");
+      tty->print_cr(" writes finals/@Stable and needs a memory barrier");
     }
   }
 

src/hotspot/share/opto/parse3.cpp

@@ -209,7 +209,21 @@ void Parse::do_put_xxx(Node* obj, ciField* field, bool is_field) {
   Node* val = type2size[bt] == 1 ? pop() : pop_pair();
 
   DecoratorSet decorators = IN_HEAP;
-  decorators |= is_vol ? MO_SEQ_CST : MO_UNORDERED;
+
+  if (is_vol) {
+    // Volatile access, full SC.
+    decorators |= MO_SEQ_CST;
+  } else if (field->is_stable() && !field->is_final() &&
+             is_reference_type(field->layout_type())) {
+    // For reference @Stable fields, make sure we publish the contents
+    // safely. We need this to make sure compilers see a proper value when
+    // constant folding the access. Final @Stable fields are already
+    // handled in constructors.
+    decorators |= MO_RELEASE;
+  } else {
+    // Everything else is unordered.
+    decorators |= MO_UNORDERED;
+  }
 
   bool is_obj = is_reference_type(bt);
 
@@ -236,22 +250,25 @@ void Parse::do_put_xxx(Node* obj, ciField* field, bool is_field) {
     set_wrote_fields(true);
 
     // If the field is final, the rules of Java say we are in <init> or <clinit>.
-    // Note the presence of writes to final non-static fields, so that we
+    // If the field is @Stable, we can be in any method, but we only care about
+    // constructors at this point.
+    //
+    // Note the presence of writes to final/@Stable non-static fields, so that we
     // can insert a memory barrier later on to keep the writes from floating
     // out of the constructor.
-    // Any method can write a @Stable field; insert memory barriers after those also.
-    if (field->is_final()) {
-      set_wrote_final(true);
+    if (field->is_final() || field->is_stable()) {
+      if (field->is_final()) {
+        set_wrote_final(true);
+      }
+      if (field->is_stable()) {
+        set_wrote_stable(true);
+      }
       if (AllocateNode::Ideal_allocation(obj) != nullptr) {
         // Preserve allocation ptr to create precedent edge to it in membar
         // generated on exit from constructor.
-        // Can't bind stable with its allocation, only record allocation for final field.
-        set_alloc_with_final(obj);
+        set_alloc_with_final_or_stable(obj);
       }
     }
-    if (field->is_stable()) {
-      set_wrote_stable(true);
-    }
   }
 }
 

src/hotspot/share/runtime/globals.hpp

@@ -1993,6 +1993,9 @@ const int ObjectAlignmentInBytes = 8;
                                                                             \
   product(bool, StressSecondarySupers, false, DIAGNOSTIC,                   \
           "Use a terrible hash function in order to generate many collisions.") \
+                                                                            \
+  develop(bool, RestrictStable, true,                                       \
+          "Restrict @Stable to trusted classes")                            \
 
 // end of RUNTIME_FLAGS