Optimism: Do not really need release-acquire for compiler side?

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_stable() ||
+       (scope()->wrote_final_or_stable() ||
          (AlwaysSafeConstructors && scope()->wrote_fields()) ||
          (support_IRIW_for_not_multiple_copy_atomic_cpu && scope()->wrote_volatile()))) {
     need_mem_bar = true;
@@ -1746,11 +1746,8 @@ void GraphBuilder::access_field(Bytecodes::Code code) {
     if (field->is_volatile()) {
       scope()->set_wrote_volatile();
     }
-    if (field->is_final()) {
-      scope()->set_wrote_final();
-    }
-    if (field->is_stable()) {
-      scope()->set_wrote_stable();
+    if (field->is_final() || field->is_stable()) {
+      scope()->set_wrote_final_or_stable();
     }
   }
 

src/hotspot/share/c1/c1_IR.cpp

@@ -143,10 +143,9 @@ IRScope::IRScope(Compilation* compilation, IRScope* caller, int caller_bci, ciMe
   _xhandlers          = new XHandlers(method);
   _number_of_locks    = 0;
   _monitor_pairing_ok = method->has_balanced_monitors();
-  _wrote_final        = false;
+  _wrote_final_or_stable = false;
   _wrote_fields       = false;
   _wrote_volatile     = false;
-  _wrote_stable       = false;
   _start              = nullptr;
 
   if (osr_bci != -1) {

src/hotspot/share/c1/c1_IR.hpp

@@ -146,10 +146,9 @@ class IRScope: public CompilationResourceObj {
   XHandlers*    _xhandlers;                      // the exception handlers
   int           _number_of_locks;                // the number of monitor lock slots needed
   bool          _monitor_pairing_ok;             // the monitor pairing info
-  bool          _wrote_final;                    // has written final field
+  bool          _wrote_final_or_stable;          // has written final or @Stable 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
@@ -182,14 +181,12 @@ class IRScope: public CompilationResourceObj {
   void          set_min_number_of_locks(int n)   { if (n > _number_of_locks) _number_of_locks = n; }
   bool          monitor_pairing_ok() const       { return _monitor_pairing_ok; }
   BlockBegin*   start() const                    { return _start; }
-  void          set_wrote_final()                { _wrote_final = true; }
-  bool          wrote_final    () const          { return _wrote_final; }
+  void          set_wrote_final_or_stable()      { _wrote_final_or_stable = true; }
+  bool          wrote_final_or_stable() const    { return _wrote_final_or_stable; }
   void          set_wrote_fields()               { _wrote_fields = true; }
   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,18 +1677,7 @@ 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,11 +78,7 @@ 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: {
-      // 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);
+      oop o = obj->obj_field(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/opto/parse.hpp

@@ -354,9 +354,8 @@ class Parse : public GraphKit {
   int           _block_count;   // Number of elements in _blocks.
 
   GraphKit      _exits;         // Record all normal returns and throws here.
-  bool          _wrote_final;   // Did we write a final field?
+  bool          _wrote_final_or_stable;   // Did we write a final or @Stable field?
   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_or_stable; // An allocation node with final or @Stable field
 
@@ -395,12 +394,10 @@ class Parse : public GraphKit {
   int           block_count()   const { return _block_count; }
 
   GraphKit&     exits()               { return _exits; }
-  bool          wrote_final() const   { return _wrote_final; }
-  void      set_wrote_final(bool z)   { _wrote_final = z; }
+  bool          wrote_final_or_stable() const   { return _wrote_final_or_stable; }
+  void      set_wrote_final_or_stable(bool z)   { _wrote_final_or_stable = z; }
   bool          wrote_volatile() const { return _wrote_volatile; }
   void      set_wrote_volatile(bool z) { _wrote_volatile = z; }
-  bool          wrote_stable() const  { return _wrote_stable; }
-  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_or_stable() const   { return _alloc_with_final_or_stable; }

src/hotspot/share/opto/parse1.cpp

@@ -408,9 +408,8 @@ Parse::Parse(JVMState* caller, ciMethod* parse_method, float expected_uses)
   _method = parse_method;
   _expected_uses = expected_uses;
   _depth = 1 + (caller->has_method() ? caller->depth() : 0);
-  _wrote_final = false;
+  _wrote_final_or_stable = false;
   _wrote_volatile = false;
-  _wrote_stable = false;
   _wrote_fields = false;
   _alloc_with_final_or_stable = nullptr;
   _block = nullptr;
@@ -1010,7 +1009,7 @@ void Parse::do_exits() {
   // exceptional returns, since they cannot publish normally.
   //
   if (method()->is_initializer() &&
-       (wrote_final() || wrote_stable() ||
+       (wrote_final_or_stable() ||
          (AlwaysSafeConstructors && wrote_fields()) ||
          (support_IRIW_for_not_multiple_copy_atomic_cpu && wrote_volatile()))) {
     Node* recorded_alloc = alloc_with_final_or_stable();

src/hotspot/share/opto/parse3.cpp

@@ -209,21 +209,7 @@ void Parse::do_put_xxx(Node* obj, ciField* field, bool is_field) {
   Node* val = type2size[bt] == 1 ? pop() : pop_pair();
 
   DecoratorSet decorators = IN_HEAP;
-
-  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;
-  }
+  decorators |= is_vol ? MO_SEQ_CST : MO_UNORDERED;
 
   bool is_obj = is_reference_type(bt);
 
@@ -257,12 +243,7 @@ void Parse::do_put_xxx(Node* obj, ciField* field, bool is_field) {
     // can insert a memory barrier later on to keep the writes from floating
     // out of the constructor.
     if (field->is_final() || field->is_stable()) {
-      if (field->is_final()) {
-        set_wrote_final(true);
-      }
-      if (field->is_stable()) {
-        set_wrote_stable(true);
-      }
+      set_wrote_final_or_stable(true);
       if (AllocateNode::Ideal_allocation(obj) != nullptr) {
         // Preserve allocation ptr to create precedent edge to it in membar
         // generated on exit from constructor.

test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java

@@ -108,10 +108,10 @@ static Carrier testConstructorInit() {
     }
 
     @Test
-    @IR(counts = { IRNode.MEMBAR, ">0" })
+    @IR(failOn = { IRNode.MEMBAR })
     static void testMethodInit() {
-        // Reference inits should have membars.
-        INIT_CARRIER.init(); // Init again.
+        // Reference inits have no membars.
+        INIT_CARRIER.init();
     }
 
 }