8349652: Rewire nmethod oop load barriers

Reviewed-by: kvn, aboldtch

src/hotspot/share/code/nmethod.cpp

@@ -2143,14 +2143,18 @@ oop nmethod::oop_at(int index) const {
   if (index == 0) {
     return nullptr;
   }
-  return NMethodAccess<AS_NO_KEEPALIVE>::oop_load(oop_addr_at(index));
+
+  BarrierSetNMethod* bs_nm = BarrierSet::barrier_set()->barrier_set_nmethod();
+  return bs_nm->oop_load_no_keepalive(this, index);
 }
 
 oop nmethod::oop_at_phantom(int index) const {
   if (index == 0) {
     return nullptr;
   }
-  return NMethodAccess<ON_PHANTOM_OOP_REF>::oop_load(oop_addr_at(index));
+
+  BarrierSetNMethod* bs_nm = BarrierSet::barrier_set()->barrier_set_nmethod();
+  return bs_nm->oop_load_phantom(this, index);
 }
 
 //

src/hotspot/share/gc/shared/barrierSetNMethod.cpp

@@ -212,3 +212,11 @@ bool BarrierSetNMethod::nmethod_osr_entry_barrier(nmethod* nm) {
   OrderAccess::cross_modify_fence();
   return result;
 }
+
+oop BarrierSetNMethod::oop_load_no_keepalive(const nmethod* nm, int index) {
+  return NativeAccess<AS_NO_KEEPALIVE>::oop_load(nm->oop_addr_at(index));
+}
+
+oop BarrierSetNMethod::oop_load_phantom(const nmethod* nm, int index) {
+  return NativeAccess<ON_PHANTOM_OOP_REF>::oop_load(nm->oop_addr_at(index));
+}

src/hotspot/share/gc/shared/barrierSetNMethod.hpp

@@ -26,6 +26,7 @@
 #define SHARE_GC_SHARED_BARRIERSETNMETHOD_HPP
 
 #include "memory/allocation.hpp"
+#include "oops/oopsHierarchy.hpp"
 #include "utilities/formatBuffer.hpp"
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/sizes.hpp"
@@ -57,6 +58,9 @@ public:
 
   void arm_all_nmethods();
 
+  virtual oop oop_load_no_keepalive(const nmethod* nm, int index);
+  virtual oop oop_load_phantom(const nmethod* nm, int index);
+
 #if INCLUDE_JVMCI
   bool verify_barrier(nmethod* nm, FormatBuffer<>& msg);
 #endif

src/hotspot/share/gc/z/zBarrierSet.inline.hpp

@@ -473,11 +473,7 @@ template <DecoratorSet decorators, typename BarrierSetT>
 inline oop ZBarrierSet::AccessBarrier<decorators, BarrierSetT>::oop_load_not_in_heap(oop* p) {
   verify_decorators_absent<ON_UNKNOWN_OOP_REF>();
 
-  if (HasDecorator<decorators, IN_NMETHOD>::value) {
-    return ZNMethod::load_oop(p, decorators);
-  } else {
-    return oop_load_not_in_heap((zpointer*)p);
-  }
+  return oop_load_not_in_heap((zpointer*)p);
 }
 
 template <DecoratorSet decorators, typename BarrierSetT>

src/hotspot/share/gc/z/zBarrierSetNMethod.cpp

@@ -97,3 +97,11 @@ int* ZBarrierSetNMethod::disarmed_guard_value_address() const {
 ByteSize ZBarrierSetNMethod::thread_disarmed_guard_value_offset() const {
   return ZThreadLocalData::nmethod_disarmed_offset();
 }
+
+oop ZBarrierSetNMethod::oop_load_no_keepalive(const nmethod* nm, int index) {
+  return ZNMethod::oop_load_no_keepalive(nm, index);
+}
+
+oop ZBarrierSetNMethod::oop_load_phantom(const nmethod* nm, int index) {
+  return ZNMethod::oop_load_phantom(nm, index);
+}

src/hotspot/share/gc/z/zBarrierSetNMethod.hpp

@@ -36,6 +36,9 @@ protected:
 public:
   virtual ByteSize thread_disarmed_guard_value_offset() const;
   virtual int* disarmed_guard_value_address() const;
+
+  virtual oop oop_load_no_keepalive(const nmethod* nm, int index);
+  virtual oop oop_load_phantom(const nmethod* nm, int index);
 };
 
 #endif // SHARE_GC_Z_ZBARRIERSETNMETHOD_HPP

src/hotspot/share/gc/z/zNMethod.cpp

@@ -305,26 +305,32 @@ uintptr_t ZNMethod::color(nmethod* nm) {
   return (uintptr_t)bs_nm->guard_value(nm);
 }
 
-oop ZNMethod::load_oop(oop* p, DecoratorSet decorators) {
-  assert((decorators & ON_WEAK_OOP_REF) == 0,
-         "nmethod oops have phantom strength, not weak");
-  nmethod* const nm = CodeCache::find_nmethod((void*)p);
-  assert(nm != nullptr, "did not find nmethod");
+oop ZNMethod::oop_load_no_keepalive(const nmethod* nm, int index) {
+  return oop_load(nm, index, false /* keep_alive */);
+}
+
+oop ZNMethod::oop_load_phantom(const nmethod* nm, int index) {
+  return oop_load(nm, index, true /* keep_alive */);
+}
+
+oop ZNMethod::oop_load(const nmethod* const_nm, int index, bool keep_alive) {
+  // The rest of the code is not ready to handle const nmethod, so cast it away
+  // until we are more consistent with our const corectness.
+  nmethod* nm = const_cast<nmethod*>(const_nm);
+
   if (!is_armed(nm)) {
     // If the nmethod entry barrier isn't armed, then it has been applied
     // already. The implication is that the contents of the memory location
     // is already a valid oop, and the barrier would have kept it alive if
     // necessary. Therefore, no action is required, and we are allowed to
     // simply read the oop.
-    return *p;
+    return *nm->oop_addr_at(index);
   }
 
-  const bool keep_alive = (decorators & ON_PHANTOM_OOP_REF) != 0 &&
-                          (decorators & AS_NO_KEEPALIVE) == 0;
   ZLocker<ZReentrantLock> locker(ZNMethod::lock_for_nmethod(nm));
 
   // Make a local root
-  zaddress_unsafe obj = *ZUncoloredRoot::cast(p);
+  zaddress_unsafe obj = *ZUncoloredRoot::cast(nm->oop_addr_at(index));
 
   if (keep_alive) {
     ZUncoloredRoot::process(&obj, ZNMethod::color(nm));

src/hotspot/share/gc/z/zNMethod.hpp

@@ -42,6 +42,8 @@ private:
   static void log_unregister(const nmethod* nm);
   static void log_purge(const nmethod* nm);
 
+  static oop oop_load(const nmethod* nm, int index, bool keep_alive);
+
 public:
   static void register_nmethod(nmethod* nm);
   static void unregister_nmethod(nmethod* nm);
@@ -69,7 +71,9 @@ public:
   static void purge();
 
   static uintptr_t color(nmethod* nm);
-  static oop load_oop(oop* p, DecoratorSet decorators);
+
+  static oop oop_load_no_keepalive(const nmethod* nm, int index);
+  static oop oop_load_phantom(const nmethod* nm, int index);
 };
 
 #endif // SHARE_GC_Z_ZNMETHOD_HPP

src/hotspot/share/oops/access.hpp

@@ -284,11 +284,6 @@ class HeapAccess: public Access<IN_HEAP | decorators> {};
 template <DecoratorSet decorators = DECORATORS_NONE>
 class NativeAccess: public Access<IN_NATIVE | decorators> {};
 
-// Helper for performing accesses in nmethods. These accesses
-// may resolve an accessor on a GC barrier set.
-template <DecoratorSet decorators = DECORATORS_NONE>
-class NMethodAccess: public Access<IN_NMETHOD | decorators> {};
-
 // Helper for array access.
 template <DecoratorSet decorators = DECORATORS_NONE>
 class ArrayAccess: public HeapAccess<IS_ARRAY | decorators> {
@@ -366,7 +361,6 @@ void Access<decorators>::verify_decorators() {
   const DecoratorSet location_decorators = decorators & IN_DECORATOR_MASK;
   STATIC_ASSERT(location_decorators == 0 || ( // make sure location decorators are disjoint if set
     (location_decorators ^ IN_NATIVE) == 0 ||
-    (location_decorators ^ IN_NMETHOD) == 0 ||
     (location_decorators ^ IN_HEAP) == 0
   ));
 }

src/hotspot/share/oops/accessDecorators.hpp

@@ -173,11 +173,9 @@ const DecoratorSet ON_DECORATOR_MASK  = ON_STRONG_OOP_REF | ON_WEAK_OOP_REF |
 // * IN_HEAP: The access is performed in the heap. Many barriers such as card marking will
 //   be omitted if this decorator is not set.
 // * IN_NATIVE: The access is performed in an off-heap data structure.
-// * IN_NMETHOD: The access is performed inside of an nmethod.
 const DecoratorSet IN_HEAP            = UCONST64(1) << 18;
 const DecoratorSet IN_NATIVE          = UCONST64(1) << 19;
-const DecoratorSet IN_NMETHOD         = UCONST64(1) << 20;
-const DecoratorSet IN_DECORATOR_MASK  = IN_HEAP | IN_NATIVE | IN_NMETHOD;
+const DecoratorSet IN_DECORATOR_MASK  = IN_HEAP | IN_NATIVE;
 
 // == Boolean Flag Decorators ==
 // * IS_ARRAY: The access is performed on a heap allocated array. This is sometimes a special case
@@ -185,9 +183,9 @@ const DecoratorSet IN_DECORATOR_MASK  = IN_HEAP | IN_NATIVE | IN_NMETHOD;
 // * IS_DEST_UNINITIALIZED: This property can be important to e.g. SATB barriers by
 //   marking that the previous value is uninitialized nonsense rather than a real value.
 // * IS_NOT_NULL: This property can make certain barriers faster such as compressing oops.
-const DecoratorSet IS_ARRAY              = UCONST64(1) << 21;
-const DecoratorSet IS_DEST_UNINITIALIZED = UCONST64(1) << 22;
-const DecoratorSet IS_NOT_NULL           = UCONST64(1) << 23;
+const DecoratorSet IS_ARRAY              = UCONST64(1) << 20;
+const DecoratorSet IS_DEST_UNINITIALIZED = UCONST64(1) << 21;
+const DecoratorSet IS_NOT_NULL           = UCONST64(1) << 22;
 
 // == Arraycopy Decorators ==
 // * ARRAYCOPY_CHECKCAST: This property means that the class of the objects in source
@@ -199,11 +197,11 @@ const DecoratorSet IS_NOT_NULL           = UCONST64(1) << 23;
 // * ARRAYCOPY_ARRAYOF: The copy is in the arrayof form.
 // * ARRAYCOPY_ATOMIC: The accesses have to be atomic over the size of its elements.
 // * ARRAYCOPY_ALIGNED: The accesses have to be aligned on a HeapWord.
-const DecoratorSet ARRAYCOPY_CHECKCAST            = UCONST64(1) << 24;
-const DecoratorSet ARRAYCOPY_DISJOINT             = UCONST64(1) << 25;
-const DecoratorSet ARRAYCOPY_ARRAYOF              = UCONST64(1) << 26;
-const DecoratorSet ARRAYCOPY_ATOMIC               = UCONST64(1) << 27;
-const DecoratorSet ARRAYCOPY_ALIGNED              = UCONST64(1) << 28;
+const DecoratorSet ARRAYCOPY_CHECKCAST            = UCONST64(1) << 23;
+const DecoratorSet ARRAYCOPY_DISJOINT             = UCONST64(1) << 24;
+const DecoratorSet ARRAYCOPY_ARRAYOF              = UCONST64(1) << 25;
+const DecoratorSet ARRAYCOPY_ATOMIC               = UCONST64(1) << 26;
+const DecoratorSet ARRAYCOPY_ALIGNED              = UCONST64(1) << 27;
 const DecoratorSet ARRAYCOPY_DECORATOR_MASK       = ARRAYCOPY_CHECKCAST | ARRAYCOPY_DISJOINT |
                                                     ARRAYCOPY_DISJOINT | ARRAYCOPY_ARRAYOF |
                                                     ARRAYCOPY_ATOMIC | ARRAYCOPY_ALIGNED;
@@ -212,11 +210,11 @@ const DecoratorSet ARRAYCOPY_DECORATOR_MASK       = ARRAYCOPY_CHECKCAST | ARRAYC
 // * ACCESS_READ: Indicate that the resolved object is accessed read-only. This allows the GC
 //   backend to use weaker and more efficient barriers.
 // * ACCESS_WRITE: Indicate that the resolved object is used for write access.
-const DecoratorSet ACCESS_READ                    = UCONST64(1) << 29;
-const DecoratorSet ACCESS_WRITE                   = UCONST64(1) << 30;
+const DecoratorSet ACCESS_READ                    = UCONST64(1) << 28;
+const DecoratorSet ACCESS_WRITE                   = UCONST64(1) << 29;
 
 // Keep track of the last decorator.
-const DecoratorSet DECORATOR_LAST = UCONST64(1) << 30;
+const DecoratorSet DECORATOR_LAST = UCONST64(1) << 29;
 
 namespace AccessInternal {
   // This class adds implied decorators that follow according to decorator rules.