changeset 57154:5e8f9713e343

8234520: ZGC: C2: Oop instance cloning causing skipped compiles Reviewed-by: pliden, vlivanov
author neliasso
date Fri, 29 Nov 2019 11:26:25 +0100
parents ae2eb76c486d
children 3636bab5e81e
files src/hotspot/share/gc/shared/c2/barrierSetC2.cpp src/hotspot/share/gc/shared/c2/barrierSetC2.hpp src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp src/hotspot/share/gc/z/zBarrierSetRuntime.cpp src/hotspot/share/gc/z/zBarrierSetRuntime.hpp src/hotspot/share/opto/arraycopynode.hpp
diffstat 6 files changed, 77 insertions(+), 30 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/gc/shared/c2/barrierSetC2.cpp	Thu Nov 28 13:02:39 2019 +0100
+++ b/src/hotspot/share/gc/shared/c2/barrierSetC2.cpp	Fri Nov 29 11:26:25 2019 +0100
@@ -644,11 +644,11 @@
   return atomic_add_at_resolved(access, new_val, value_type);
 }
 
-void BarrierSetC2::clone(GraphKit* kit, Node* src, Node* dst, Node* size, bool is_array) const {
+int BarrierSetC2::arraycopy_payload_base_offset(bool is_array) {
   // Exclude the header but include array length to copy by 8 bytes words.
   // Can't use base_offset_in_bytes(bt) since basic type is unknown.
   int base_off = is_array ? arrayOopDesc::length_offset_in_bytes() :
-                            instanceOopDesc::base_offset_in_bytes();
+                 instanceOopDesc::base_offset_in_bytes();
   // base_off:
   // 8  - 32-bit VM
   // 12 - 64-bit VM, compressed klass
@@ -664,18 +664,27 @@
     }
     assert(base_off % BytesPerLong == 0, "expect 8 bytes alignment");
   }
-  Node* src_base  = kit->basic_plus_adr(src,  base_off);
-  Node* dst_base = kit->basic_plus_adr(dst, base_off);
+  return base_off;
+}
+
+void BarrierSetC2::clone(GraphKit* kit, Node* src_base, Node* dst_base, Node* size, bool is_array) const {
+  int base_off = arraycopy_payload_base_offset(is_array);
+  Node* payload_src = kit->basic_plus_adr(src_base,  base_off);
+  Node* payload_dst = kit->basic_plus_adr(dst_base, base_off);
 
   // Compute the length also, if needed:
-  Node* countx = size;
-  countx = kit->gvn().transform(new SubXNode(countx, kit->MakeConX(base_off)));
-  countx = kit->gvn().transform(new URShiftXNode(countx, kit->intcon(LogBytesPerLong) ));
+  Node* payload_size = size;
+  payload_size = kit->gvn().transform(new SubXNode(payload_size, kit->MakeConX(base_off)));
+  payload_size = kit->gvn().transform(new URShiftXNode(payload_size, kit->intcon(LogBytesPerLong) ));
 
   const TypePtr* raw_adr_type = TypeRawPtr::BOTTOM;
 
-  ArrayCopyNode* ac = ArrayCopyNode::make(kit, false, src_base, NULL, dst_base, NULL, countx, true, false);
-  ac->set_clonebasic();
+  ArrayCopyNode* ac = ArrayCopyNode::make(kit, false, payload_src, NULL, payload_dst, NULL, payload_size, true, false);
+  if (is_array) {
+    ac->set_clone_array();
+  } else {
+    ac->set_clone_inst();
+  }
   Node* n = kit->gvn().transform(ac);
   if (n == ac) {
     ac->_adr_type = TypeRawPtr::BOTTOM;
--- a/src/hotspot/share/gc/shared/c2/barrierSetC2.hpp	Thu Nov 28 13:02:39 2019 +0100
+++ b/src/hotspot/share/gc/shared/c2/barrierSetC2.hpp	Fri Nov 29 11:26:25 2019 +0100
@@ -325,6 +325,8 @@
   virtual void late_barrier_analysis() const { }
   virtual int estimate_stub_size() const { return 0; }
   virtual void emit_stubs(CodeBuffer& cb) const { }
+
+  static int arraycopy_payload_base_offset(bool is_array);
 };
 
 #endif // SHARE_GC_SHARED_C2_BARRIERSETC2_HPP
--- a/src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp	Thu Nov 28 13:02:39 2019 +0100
+++ b/src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp	Fri Nov 29 11:26:25 2019 +0100
@@ -28,6 +28,7 @@
 #include "gc/z/zBarrierSetAssembler.hpp"
 #include "gc/z/zBarrierSetRuntime.hpp"
 #include "opto/arraycopynode.hpp"
+#include "opto/addnode.hpp"
 #include "opto/block.hpp"
 #include "opto/compile.hpp"
 #include "opto/graphKit.hpp"
@@ -215,13 +216,15 @@
   return type == T_OBJECT || type == T_ARRAY;
 }
 
+// This TypeFunc assumes a 64bit system
 static const TypeFunc* clone_type() {
   // Create input type (domain)
-  const Type** domain_fields = TypeTuple::fields(3);
+  const Type** domain_fields = TypeTuple::fields(4);
   domain_fields[TypeFunc::Parms + 0] = TypeInstPtr::NOTNULL;  // src
   domain_fields[TypeFunc::Parms + 1] = TypeInstPtr::NOTNULL;  // dst
-  domain_fields[TypeFunc::Parms + 2] = TypeInt::INT;          // size
-  const TypeTuple* domain = TypeTuple::make(TypeFunc::Parms + 3, domain_fields);
+  domain_fields[TypeFunc::Parms + 2] = TypeLong::LONG;        // size lower
+  domain_fields[TypeFunc::Parms + 3] = Type::HALF;            // size upper
+  const TypeTuple* domain = TypeTuple::make(TypeFunc::Parms + 4, domain_fields);
 
   // Create result type (range)
   const Type** range_fields = TypeTuple::fields(0);
@@ -230,27 +233,51 @@
   return TypeFunc::make(domain, range);
 }
 
+// Node n is pointing to the start of oop payload - return base pointer
+static Node* get_base_for_arracycopy_clone(PhaseMacroExpand* phase, Node* n) {
+  // This would normally be handled by optimizations, but the type system
+  // checks get confused when it thinks it already has a base pointer.
+  const int base_offset = BarrierSetC2::arraycopy_payload_base_offset(false);
+
+  if (n->is_AddP() &&
+      n->in(AddPNode::Offset)->is_Con() &&
+      n->in(AddPNode::Offset)->get_long() == base_offset) {
+    assert(n->in(AddPNode::Base) == n->in(AddPNode::Address), "Sanity check");
+    return n->in(AddPNode::Base);
+  } else {
+    return phase->basic_plus_adr(n, phase->longcon(-base_offset));
+  }
+}
+
 void ZBarrierSetC2::clone_at_expansion(PhaseMacroExpand* phase, ArrayCopyNode* ac) const {
   Node* const src = ac->in(ArrayCopyNode::Src);
-
-  if (src->bottom_type()->isa_aryptr()) {
+  if (ac->is_clone_array()) {
     // Clone primitive array
     BarrierSetC2::clone_at_expansion(phase, ac);
     return;
   }
 
   // Clone instance
-  Node* const ctrl = ac->in(TypeFunc::Control);
-  Node* const mem = ac->in(TypeFunc::Memory);
-  Node* const dst = ac->in(ArrayCopyNode::Dest);
+  assert(ac->is_clone_inst(), "Sanity check");
+
+  Node* const ctrl       = ac->in(TypeFunc::Control);
+  Node* const mem        = ac->in(TypeFunc::Memory);
+  Node* const dst        = ac->in(ArrayCopyNode::Dest);
   Node* const src_offset = ac->in(ArrayCopyNode::SrcPos);
   Node* const dst_offset = ac->in(ArrayCopyNode::DestPos);
-  Node* const size = ac->in(ArrayCopyNode::Length);
+  Node* const size       = ac->in(ArrayCopyNode::Length);
 
-  assert(src->bottom_type()->isa_instptr(), "Should be an instance");
-  assert(dst->bottom_type()->isa_instptr(), "Should be an instance");
   assert(src_offset == NULL, "Should be null");
   assert(dst_offset == NULL, "Should be null");
+  assert(size->bottom_type()->is_long(), "Should be long");
+
+  // The src and dst point to the object payload rather than the object base
+  Node* const src_base = get_base_for_arracycopy_clone(phase, src);
+  Node* const dst_base = get_base_for_arracycopy_clone(phase, dst);
+
+  // The size must also be increased to match the instance size
+  Node* const base_offset = phase->longcon(arraycopy_payload_base_offset(false) >> LogBytesPerLong);
+  Node* const full_size = phase->transform_later(new AddLNode(size, base_offset));
 
   Node* const call = phase->make_leaf_call(ctrl,
                                            mem,
@@ -258,9 +285,10 @@
                                            ZBarrierSetRuntime::clone_addr(),
                                            "ZBarrierSetRuntime::clone",
                                            TypeRawPtr::BOTTOM,
-                                           src,
-                                           dst,
-                                           size);
+                                           src_base,
+                                           dst_base,
+                                           full_size,
+                                           phase->top());
   phase->transform_later(call);
   phase->igvn().replace_node(ac, call);
 }
@@ -342,7 +370,7 @@
       MachNode* mem = mem_ops.at(j)->as_Mach();
       const TypePtr* mem_adr_type = NULL;
       intptr_t mem_offset = 0;
-      const Node* mem_obj = mem_obj = mem->get_base_and_disp(mem_offset, mem_adr_type);
+      const Node* mem_obj = mem->get_base_and_disp(mem_offset, mem_adr_type);
       Block* mem_block = cfg->get_block_for_node(mem);
       uint mem_index = block_index(mem_block, mem);
 
--- a/src/hotspot/share/gc/z/zBarrierSetRuntime.cpp	Thu Nov 28 13:02:39 2019 +0100
+++ b/src/hotspot/share/gc/z/zBarrierSetRuntime.cpp	Fri Nov 29 11:26:25 2019 +0100
@@ -42,7 +42,7 @@
   ZBarrier::load_barrier_on_oop_array(p, length);
 JRT_END
 
-JRT_LEAF(void, ZBarrierSetRuntime::clone(oop src, oop dst, size_t size))
+JRT_LEAF(void, ZBarrierSetRuntime::clone(oopDesc* src, oopDesc* dst, size_t size))
   HeapAccess<>::clone(src, dst, size);
 JRT_END
 
--- a/src/hotspot/share/gc/z/zBarrierSetRuntime.hpp	Thu Nov 28 13:02:39 2019 +0100
+++ b/src/hotspot/share/gc/z/zBarrierSetRuntime.hpp	Fri Nov 29 11:26:25 2019 +0100
@@ -36,7 +36,7 @@
   static oopDesc* load_barrier_on_weak_oop_field_preloaded(oopDesc* o, oop* p);
   static oopDesc* load_barrier_on_phantom_oop_field_preloaded(oopDesc* o, oop* p);
   static void load_barrier_on_oop_array(oop* p, size_t length);
-  static void clone(oop src, oop dst, size_t size);
+  static void clone(oopDesc* src, oopDesc* dst, size_t size);
 
 public:
   static address load_barrier_on_oop_field_preloaded_addr(DecoratorSet decorators);
--- a/src/hotspot/share/opto/arraycopynode.hpp	Thu Nov 28 13:02:39 2019 +0100
+++ b/src/hotspot/share/opto/arraycopynode.hpp	Fri Nov 29 11:26:25 2019 +0100
@@ -37,8 +37,10 @@
   enum {
     None,            // not set yet
     ArrayCopy,       // System.arraycopy()
-    CloneBasic,      // A clone that can be copied by 64 bit chunks
-    CloneOopArray,   // An oop array clone
+    CloneInst,       // A clone of instances
+    CloneArray,      // A clone of arrays that don't require a barrier
+                     // - depends on GC - some need to treat oop arrays separately
+    CloneOopArray,   // An oop array clone that requires GC barriers
     CopyOf,          // Arrays.copyOf()
     CopyOfRange      // Arrays.copyOfRange()
   } _kind;
@@ -146,15 +148,21 @@
 
   bool is_arraycopy()             const  { assert(_kind != None, "should bet set"); return _kind == ArrayCopy; }
   bool is_arraycopy_validated()   const  { assert(_kind != None, "should bet set"); return _kind == ArrayCopy && _arguments_validated; }
-  bool is_clonebasic()            const  { assert(_kind != None, "should bet set"); return _kind == CloneBasic; }
+  bool is_clone_inst()            const  { assert(_kind != None, "should bet set"); return _kind == CloneInst; }
+  // is_clone_array - true for all arrays when using GCs that has no barriers
+  bool is_clone_array()           const  { assert(_kind != None, "should bet set"); return _kind == CloneArray; }
+  // is_clone_oop_array is used when oop arrays need GC barriers
   bool is_clone_oop_array()       const  { assert(_kind != None, "should bet set"); return _kind == CloneOopArray; }
+  // is_clonebasic - is true for any type of clone that doesn't need a barrier.
+  bool is_clonebasic()            const  { assert(_kind != None, "should bet set"); return _kind == CloneInst || _kind == CloneArray; }
   bool is_copyof()                const  { assert(_kind != None, "should bet set"); return _kind == CopyOf; }
   bool is_copyof_validated()      const  { assert(_kind != None, "should bet set"); return _kind == CopyOf && _arguments_validated; }
   bool is_copyofrange()           const  { assert(_kind != None, "should bet set"); return _kind == CopyOfRange; }
   bool is_copyofrange_validated() const  { assert(_kind != None, "should bet set"); return _kind == CopyOfRange && _arguments_validated; }
 
   void set_arraycopy(bool validated)   { assert(_kind == None, "shouldn't bet set yet"); _kind = ArrayCopy; _arguments_validated = validated; }
-  void set_clonebasic()                { assert(_kind == None, "shouldn't bet set yet"); _kind = CloneBasic; }
+  void set_clone_inst()                { assert(_kind == None, "shouldn't bet set yet"); _kind = CloneInst; }
+  void set_clone_array()               { assert(_kind == None, "shouldn't bet set yet"); _kind = CloneArray; }
   void set_clone_oop_array()           { assert(_kind == None, "shouldn't bet set yet"); _kind = CloneOopArray; }
   void set_copyof(bool validated)      { assert(_kind == None, "shouldn't bet set yet"); _kind = CopyOf; _arguments_validated = validated; }
   void set_copyofrange(bool validated) { assert(_kind == None, "shouldn't bet set yet"); _kind = CopyOfRange; _arguments_validated = validated; }