8269897: Shenandoah: Resolve UNKNOWN access strength, where possible

Reviewed-by: shade

Author: rkennke

src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.hpp

@@ -54,11 +54,11 @@ class ShenandoahBarrierSet: public BarrierSet {
   static bool need_keep_alive_barrier(DecoratorSet decorators, BasicType type);
 
   static bool is_strong_access(DecoratorSet decorators) {
-    return (decorators & (ON_WEAK_OOP_REF | ON_PHANTOM_OOP_REF | ON_UNKNOWN_OOP_REF)) == 0;
+    return (decorators & (ON_WEAK_OOP_REF | ON_PHANTOM_OOP_REF)) == 0;
   }
 
   static bool is_weak_access(DecoratorSet decorators) {
-    return (decorators & (ON_WEAK_OOP_REF | ON_UNKNOWN_OOP_REF)) != 0;
+    return (decorators & ON_WEAK_OOP_REF) != 0;
   }
 
   static bool is_phantom_access(DecoratorSet decorators) {
@@ -90,8 +90,6 @@ class ShenandoahBarrierSet: public BarrierSet {
   inline void satb_enqueue(oop value);
   inline void iu_barrier(oop obj);
 
-  template <DecoratorSet decorators>
-  inline void keep_alive_if_weak(oop value);
   inline void keep_alive_if_weak(DecoratorSet decorators, oop value);
 
   inline void enqueue(oop obj);
@@ -101,8 +99,17 @@ class ShenandoahBarrierSet: public BarrierSet {
   template <class T>
   inline oop load_reference_barrier_mutator(oop obj, T* load_addr);
 
-  template <DecoratorSet decorators, class T>
-  inline oop load_reference_barrier(oop obj, T* load_addr);
+  template <class T>
+  inline oop load_reference_barrier(DecoratorSet decorators, oop obj, T* load_addr);
+
+  template <typename T>
+  inline oop oop_load(DecoratorSet decorators, T* addr);
+
+  template <typename T>
+  inline oop oop_cmpxchg(DecoratorSet decorators, T* addr, oop compare_value, oop new_value);
+
+  template <typename T>
+  inline oop oop_xchg(DecoratorSet decorators, T* addr, oop new_value);
 
 private:
   template <class T>

src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp

@@ -99,29 +99,29 @@ inline oop ShenandoahBarrierSet::load_reference_barrier(oop obj) {
   return obj;
 }
 
-template <DecoratorSet decorators, class T>
-inline oop ShenandoahBarrierSet::load_reference_barrier(oop obj, T* load_addr) {
+template <class T>
+inline oop ShenandoahBarrierSet::load_reference_barrier(DecoratorSet decorators, oop obj, T* load_addr) {
   if (obj == NULL) {
     return NULL;
   }
 
   // Prevent resurrection of unreachable phantom (i.e. weak-native) references.
-  if (HasDecorator<decorators, ON_PHANTOM_OOP_REF>::value &&
+  if ((decorators & ON_PHANTOM_OOP_REF) != 0 &&
       _heap->is_concurrent_weak_root_in_progress() &&
       !_heap->marking_context()->is_marked(obj)) {
     return NULL;
   }
 
   // Prevent resurrection of unreachable weak references.
-  if ((HasDecorator<decorators, ON_WEAK_OOP_REF>::value || HasDecorator<decorators, ON_UNKNOWN_OOP_REF>::value) &&
+  if ((decorators & ON_WEAK_OOP_REF) != 0 &&
       _heap->is_concurrent_weak_root_in_progress() &&
       !_heap->marking_context()->is_marked_strong(obj)) {
     return NULL;
   }
 
   // Prevent resurrection of unreachable objects that are visited during
   // concurrent class-unloading.
-  if (HasDecorator<decorators, AS_NO_KEEPALIVE>::value &&
+  if ((decorators & AS_NO_KEEPALIVE) != 0 &&
       _heap->is_evacuation_in_progress() &&
       !_heap->marking_context()->is_marked(obj)) {
     return obj;
@@ -184,45 +184,64 @@ inline void ShenandoahBarrierSet::keep_alive_if_weak(DecoratorSet decorators, oo
   }
 }
 
-template <DecoratorSet decorators>
-inline void ShenandoahBarrierSet::keep_alive_if_weak(oop value) {
-  assert((decorators & ON_UNKNOWN_OOP_REF) == 0, "Reference strength must be known");
-  if (!HasDecorator<decorators, ON_STRONG_OOP_REF>::value &&
-      !HasDecorator<decorators, AS_NO_KEEPALIVE>::value) {
-    satb_enqueue(value);
-  }
+template <typename T>
+inline oop ShenandoahBarrierSet::oop_load(DecoratorSet decorators, T* addr) {
+  oop value = RawAccess<>::oop_load(addr);
+  value = load_reference_barrier(decorators, value, addr);
+  keep_alive_if_weak(decorators, value);
+  return value;
+}
+
+template <typename T>
+inline oop ShenandoahBarrierSet::oop_cmpxchg(DecoratorSet decorators, T* addr, oop compare_value, oop new_value) {
+  iu_barrier(new_value);
+  oop res;
+  oop expected = compare_value;
+  do {
+    compare_value = expected;
+    res = RawAccess<>::oop_atomic_cmpxchg(addr, compare_value, new_value);
+    expected = res;
+  } while ((compare_value != expected) && (resolve_forwarded(compare_value) == resolve_forwarded(expected)));
+
+  // Note: We don't need a keep-alive-barrier here. We already enqueue any loaded reference for SATB anyway,
+  // because it must be the previous value.
+  res = load_reference_barrier(decorators, res, reinterpret_cast<T*>(NULL));
+  satb_enqueue(res);
+  return res;
+}
+
+template <typename T>
+inline oop ShenandoahBarrierSet::oop_xchg(DecoratorSet decorators, T* addr, oop new_value) {
+  iu_barrier(new_value);
+  oop previous = RawAccess<>::oop_atomic_xchg(addr, new_value);
+  // Note: We don't need a keep-alive-barrier here. We already enqueue any loaded reference for SATB anyway,
+  // because it must be the previous value.
+  previous = load_reference_barrier<T>(decorators, previous, reinterpret_cast<T*>(NULL));
+  satb_enqueue(previous);
+  return previous;
 }
 
 template <DecoratorSet decorators, typename BarrierSetT>
 template <typename T>
 inline oop ShenandoahBarrierSet::AccessBarrier<decorators, BarrierSetT>::oop_load_not_in_heap(T* addr) {
-  oop value = Raw::oop_load_not_in_heap(addr);
-  if (value != NULL) {
-    ShenandoahBarrierSet *const bs = ShenandoahBarrierSet::barrier_set();
-    value = bs->load_reference_barrier<decorators, T>(value, addr);
-    bs->keep_alive_if_weak<decorators>(value);
-  }
-  return value;
+  assert((decorators & ON_UNKNOWN_OOP_REF) == 0, "must be absent");
+  ShenandoahBarrierSet* const bs = ShenandoahBarrierSet::barrier_set();
+  return bs->oop_load(decorators, addr);
 }
 
 template <DecoratorSet decorators, typename BarrierSetT>
 template <typename T>
 inline oop ShenandoahBarrierSet::AccessBarrier<decorators, BarrierSetT>::oop_load_in_heap(T* addr) {
-  oop value = Raw::oop_load_in_heap(addr);
-  ShenandoahBarrierSet *const bs = ShenandoahBarrierSet::barrier_set();
-  value = bs->load_reference_barrier<decorators, T>(value, addr);
-  bs->keep_alive_if_weak<decorators>(value);
-  return value;
+  assert((decorators & ON_UNKNOWN_OOP_REF) == 0, "must be absent");
+  ShenandoahBarrierSet* const bs = ShenandoahBarrierSet::barrier_set();
+  return bs->oop_load(decorators, addr);
 }
 
 template <DecoratorSet decorators, typename BarrierSetT>
 inline oop ShenandoahBarrierSet::AccessBarrier<decorators, BarrierSetT>::oop_load_in_heap_at(oop base, ptrdiff_t offset) {
-  oop value = Raw::oop_load_in_heap_at(base, offset);
-  ShenandoahBarrierSet *const bs = ShenandoahBarrierSet::barrier_set();
+  ShenandoahBarrierSet* const bs = ShenandoahBarrierSet::barrier_set();
   DecoratorSet resolved_decorators = AccessBarrierSupport::resolve_possibly_unknown_oop_ref_strength<decorators>(base, offset);
-  value = bs->load_reference_barrier<decorators>(value, AccessInternal::oop_field_addr<decorators>(base, offset));
-  bs->keep_alive_if_weak(resolved_decorators, value);
-  return value;
+  return bs->oop_load(resolved_decorators, AccessInternal::oop_field_addr<decorators>(base, offset));
 }
 
 template <DecoratorSet decorators, typename BarrierSetT>
@@ -254,59 +273,49 @@ inline void ShenandoahBarrierSet::AccessBarrier<decorators, BarrierSetT>::oop_st
 template <DecoratorSet decorators, typename BarrierSetT>
 template <typename T>
 inline oop ShenandoahBarrierSet::AccessBarrier<decorators, BarrierSetT>::oop_atomic_cmpxchg_not_in_heap(T* addr, oop compare_value, oop new_value) {
+  assert((decorators & (AS_NO_KEEPALIVE | ON_UNKNOWN_OOP_REF)) == 0, "must be absent");
   ShenandoahBarrierSet* bs = ShenandoahBarrierSet::barrier_set();
-  bs->iu_barrier(new_value);
-
-  oop res;
-  oop expected = compare_value;
-  do {
-    compare_value = expected;
-    res = Raw::oop_atomic_cmpxchg(addr, compare_value, new_value);
-    expected = res;
-  } while ((compare_value != expected) && (resolve_forwarded(compare_value) == resolve_forwarded(expected)));
-
-  // Note: We don't need a keep-alive-barrier here. We already enqueue any loaded reference for SATB anyway,
-  // because it must be the previous value.
-  res = ShenandoahBarrierSet::barrier_set()->load_reference_barrier<decorators, T>(res, NULL);
-  bs->satb_enqueue(res);
-  return res;
+  return bs->oop_cmpxchg(decorators, addr, compare_value, new_value);
 }
 
 template <DecoratorSet decorators, typename BarrierSetT>
 template <typename T>
 inline oop ShenandoahBarrierSet::AccessBarrier<decorators, BarrierSetT>::oop_atomic_cmpxchg_in_heap(T* addr, oop compare_value, oop new_value) {
-  return oop_atomic_cmpxchg_not_in_heap(addr, compare_value, new_value);
+  assert((decorators & (AS_NO_KEEPALIVE | ON_UNKNOWN_OOP_REF)) == 0, "must be absent");
+  ShenandoahBarrierSet* bs = ShenandoahBarrierSet::barrier_set();
+  return bs->oop_cmpxchg(decorators, addr, compare_value, new_value);
 }
 
 template <DecoratorSet decorators, typename BarrierSetT>
 inline oop ShenandoahBarrierSet::AccessBarrier<decorators, BarrierSetT>::oop_atomic_cmpxchg_in_heap_at(oop base, ptrdiff_t offset, oop compare_value, oop new_value) {
-  return oop_atomic_cmpxchg_in_heap(AccessInternal::oop_field_addr<decorators>(base, offset), compare_value, new_value);
+  assert((decorators & AS_NO_KEEPALIVE) == 0, "must be absent");
+  ShenandoahBarrierSet* bs = ShenandoahBarrierSet::barrier_set();
+  DecoratorSet resolved_decorators = AccessBarrierSupport::resolve_possibly_unknown_oop_ref_strength<decorators>(base, offset);
+  return bs->oop_cmpxchg(resolved_decorators, AccessInternal::oop_field_addr<decorators>(base, offset), compare_value, new_value);
 }
 
 template <DecoratorSet decorators, typename BarrierSetT>
 template <typename T>
 inline oop ShenandoahBarrierSet::AccessBarrier<decorators, BarrierSetT>::oop_atomic_xchg_not_in_heap(T* addr, oop new_value) {
+  assert((decorators & (AS_NO_KEEPALIVE | ON_UNKNOWN_OOP_REF)) == 0, "must be absent");
   ShenandoahBarrierSet* bs = ShenandoahBarrierSet::barrier_set();
-  bs->iu_barrier(new_value);
-
-  oop previous = Raw::oop_atomic_xchg(addr, new_value);
-
-  // Note: We don't need a keep-alive-barrier here. We already enqueue any loaded reference for SATB anyway,
-  // because it must be the previous value.
-  previous = ShenandoahBarrierSet::barrier_set()->load_reference_barrier<decorators, T>(previous, NULL);
-  bs->satb_enqueue(previous);
-  return previous;
+  return bs->oop_xchg(decorators, addr, new_value);
 }
 
 template <DecoratorSet decorators, typename BarrierSetT>
 template <typename T>
 inline oop ShenandoahBarrierSet::AccessBarrier<decorators, BarrierSetT>::oop_atomic_xchg_in_heap(T* addr, oop new_value) {
-  return oop_atomic_xchg_not_in_heap(addr, new_value);
+  assert((decorators & (AS_NO_KEEPALIVE | ON_UNKNOWN_OOP_REF)) == 0, "must be absent");
+  ShenandoahBarrierSet* bs = ShenandoahBarrierSet::barrier_set();
+  return bs->oop_xchg(decorators, addr, new_value);
 }
 
 template <DecoratorSet decorators, typename BarrierSetT>
 inline oop ShenandoahBarrierSet::AccessBarrier<decorators, BarrierSetT>::oop_atomic_xchg_in_heap_at(oop base, ptrdiff_t offset, oop new_value) {
-  return oop_atomic_xchg_in_heap(AccessInternal::oop_field_addr<decorators>(base, offset), new_value);
+  assert((decorators & AS_NO_KEEPALIVE) == 0, "must be absent");
+  ShenandoahBarrierSet* bs = ShenandoahBarrierSet::barrier_set();
+  DecoratorSet resolved_decorators = AccessBarrierSupport::resolve_possibly_unknown_oop_ref_strength<decorators>(base, offset);
+  return bs->oop_xchg(resolved_decorators, AccessInternal::oop_field_addr<decorators>(base, offset), new_value);
 }
 
 // Clone barrier support

src/hotspot/share/gc/shenandoah/shenandoahRuntime.cpp

@@ -68,17 +68,17 @@ JRT_LEAF(void, ShenandoahRuntime::shenandoah_clone_barrier(oopDesc* src))
 JRT_END
 
 JRT_LEAF(oopDesc*, ShenandoahRuntime::load_reference_barrier_weak(oopDesc * src, oop* load_addr))
-  return (oopDesc*) ShenandoahBarrierSet::barrier_set()->load_reference_barrier<ON_WEAK_OOP_REF, oop>(oop(src), load_addr);
+  return (oopDesc*) ShenandoahBarrierSet::barrier_set()->load_reference_barrier<oop>(ON_WEAK_OOP_REF, oop(src), load_addr);
 JRT_END
 
 JRT_LEAF(oopDesc*, ShenandoahRuntime::load_reference_barrier_weak_narrow(oopDesc * src, narrowOop* load_addr))
-  return (oopDesc*) ShenandoahBarrierSet::barrier_set()->load_reference_barrier<ON_WEAK_OOP_REF, narrowOop>(oop(src), load_addr);
+  return (oopDesc*) ShenandoahBarrierSet::barrier_set()->load_reference_barrier<narrowOop>(ON_WEAK_OOP_REF, oop(src), load_addr);
 JRT_END
 
 JRT_LEAF(oopDesc*, ShenandoahRuntime::load_reference_barrier_phantom(oopDesc * src, oop* load_addr))
-  return (oopDesc*) ShenandoahBarrierSet::barrier_set()->load_reference_barrier<ON_PHANTOM_OOP_REF, oop>(oop(src), load_addr);
+  return (oopDesc*) ShenandoahBarrierSet::barrier_set()->load_reference_barrier<oop>(ON_PHANTOM_OOP_REF, oop(src), load_addr);
 JRT_END
 
 JRT_LEAF(oopDesc*, ShenandoahRuntime::load_reference_barrier_phantom_narrow(oopDesc * src, narrowOop* load_addr))
-  return (oopDesc*) ShenandoahBarrierSet::barrier_set()->load_reference_barrier<ON_PHANTOM_OOP_REF, narrowOop>(oop(src), load_addr);
+  return (oopDesc*) ShenandoahBarrierSet::barrier_set()->load_reference_barrier<narrowOop>(ON_PHANTOM_OOP_REF, oop(src), load_addr);
 JRT_END