8238759: Clones should always keep the base pointer

Reviewed-by: rkennke, thartmann

src/hotspot/share/gc/shared/c2/barrierSetC2.cpp

@@ -653,7 +653,7 @@ 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
@@ -674,24 +674,19 @@ int BarrierSetC2::arraycopy_payload_base_offset(bool is_array) {
 
 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* 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, payload_src, NULL, payload_dst, NULL, payload_size, true, false);
+  Node* offset = kit->MakeConX(base_off);
+  payload_size = kit->gvn().transform(new SubXNode(payload_size, offset));
+  payload_size = kit->gvn().transform(new URShiftXNode(payload_size, kit->intcon(LogBytesPerLong)));
+  ArrayCopyNode* ac = ArrayCopyNode::make(kit, false, src_base, offset,  dst_base, offset, 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) {
+    const TypePtr* raw_adr_type = TypeRawPtr::BOTTOM;
     ac->_adr_type = TypeRawPtr::BOTTOM;
     kit->set_predefined_output_for_runtime_call(ac, ac->in(TypeFunc::Memory), raw_adr_type);
   } else {
@@ -837,18 +832,16 @@ void BarrierSetC2::clone_at_expansion(PhaseMacroExpand* phase, ArrayCopyNode* ac
   Node* dest_offset = ac->in(ArrayCopyNode::DestPos);
   Node* length = ac->in(ArrayCopyNode::Length);
 
-  assert (src_offset == NULL,  "for clone offsets should be null");
-  assert (dest_offset == NULL, "for clone offsets should be null");
+  Node* payload_src = phase->basic_plus_adr(src, src_offset);
+  Node* payload_dst = phase->basic_plus_adr(dest, dest_offset);
 
   const char* copyfunc_name = "arraycopy";
-  address     copyfunc_addr =
-          phase->basictype2arraycopy(T_LONG, NULL, NULL,
-                              true, copyfunc_name, true);
+  address     copyfunc_addr = phase->basictype2arraycopy(T_LONG, NULL, NULL, true, copyfunc_name, true);
 
   const TypePtr* raw_adr_type = TypeRawPtr::BOTTOM;
   const TypeFunc* call_type = OptoRuntime::fast_arraycopy_Type();
 
-  Node* call = phase->make_leaf_call(ctrl, mem, call_type, copyfunc_addr, copyfunc_name, raw_adr_type, src, dest, length XTOP);
+  Node* call = phase->make_leaf_call(ctrl, mem, call_type, copyfunc_addr, copyfunc_name, raw_adr_type, payload_src, payload_dst, length XTOP);
   phase->transform_later(call);
 
   phase->igvn().replace_node(ac, call);

src/hotspot/share/gc/shenandoah/c2/shenandoahBarrierSetC2.cpp

@@ -816,14 +816,14 @@ bool ShenandoahBarrierSetC2::clone_needs_barrier(Node* src, PhaseGVN& gvn) {
 void ShenandoahBarrierSetC2::clone_at_expansion(PhaseMacroExpand* phase, ArrayCopyNode* ac) const {
   Node* ctrl = ac->in(TypeFunc::Control);
   Node* mem = ac->in(TypeFunc::Memory);
-  Node* src = ac->in(ArrayCopyNode::Src);
+  Node* src_base = ac->in(ArrayCopyNode::Src);
   Node* src_offset = ac->in(ArrayCopyNode::SrcPos);
-  Node* dest = ac->in(ArrayCopyNode::Dest);
+  Node* dest_base = ac->in(ArrayCopyNode::Dest);
   Node* dest_offset = ac->in(ArrayCopyNode::DestPos);
   Node* length = ac->in(ArrayCopyNode::Length);
-  assert (src_offset == NULL && dest_offset == NULL, "for clone offsets should be null");
-  assert (src->is_AddP(), "for clone the src should be the interior ptr");
-  assert (dest->is_AddP(), "for clone the dst should be the interior ptr");
+
+  Node* src = phase->basic_plus_adr(src_base, src_offset);
+  Node* dest = phase->basic_plus_adr(dest_base, dest_offset);
 
   if (ShenandoahCloneBarrier && clone_needs_barrier(src, phase->igvn())) {
     // Check if heap is has forwarded objects. If it does, we need to call into the special
@@ -860,7 +860,7 @@ void ShenandoahBarrierSetC2::clone_at_expansion(PhaseMacroExpand* phase, ArrayCo
                     CAST_FROM_FN_PTR(address, ShenandoahRuntime::shenandoah_clone_barrier),
                     "shenandoah_clone",
                     TypeRawPtr::BOTTOM,
-                    src->in(AddPNode::Base));
+                    src_base);
     call = phase->transform_later(call);
 
     ctrl = phase->transform_later(new ProjNode(call, TypeFunc::Control));

src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp

@@ -233,22 +233,6 @@ static const TypeFunc* clone_type() {
   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 (ac->is_clone_array()) {
@@ -258,24 +242,16 @@ void ZBarrierSetC2::clone_at_expansion(PhaseMacroExpand* phase, ArrayCopyNode* a
   }
 
   // Clone instance
-  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);
 
-  assert(src_offset == NULL, "Should be null");
-  assert(dst_offset == NULL, "Should be null");
+  assert(ac->is_clone_inst(), "Sanity check");
   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
+  // The native clone we are calling here expects the instance size in words
+  // Add header/offset size to payload size to get 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));
 
@@ -285,8 +261,8 @@ void ZBarrierSetC2::clone_at_expansion(PhaseMacroExpand* phase, ArrayCopyNode* a
                                            ZBarrierSetRuntime::clone_addr(),
                                            "ZBarrierSetRuntime::clone",
                                            TypeRawPtr::BOTTOM,
-                                           src_base,
-                                           dst_base,
+                                           src,
+                                           dst,
                                            full_size,
                                            phase->top());
   phase->transform_later(call);

src/hotspot/share/opto/arraycopynode.cpp

@@ -57,7 +57,7 @@ ArrayCopyNode* ArrayCopyNode::make(GraphKit* kit, bool may_throw,
                                    Node* src_length, Node* dest_length) {
 
   ArrayCopyNode* ac = new ArrayCopyNode(kit->C, alloc_tightly_coupled, has_negative_length_guard);
-  Node* prev_mem = kit->set_predefined_input_for_runtime_call(ac);
+  kit->set_predefined_input_for_runtime_call(ac);
 
   ac->init_req(ArrayCopyNode::Src, src);
   ac->init_req(ArrayCopyNode::SrcPos, src_offset);
@@ -176,17 +176,12 @@ Node* ArrayCopyNode::try_clone_instance(PhaseGVN *phase, bool can_reshape, int c
     return NULL;
   }
 
-  Node* src = in(ArrayCopyNode::Src);
-  Node* dest = in(ArrayCopyNode::Dest);
+  Node* base_src = in(ArrayCopyNode::Src);
+  Node* base_dest = in(ArrayCopyNode::Dest);
   Node* ctl = in(TypeFunc::Control);
   Node* in_mem = in(TypeFunc::Memory);
 
-  const Type* src_type = phase->type(src);
-
-  assert(src->is_AddP(), "should be base + off");
-  assert(dest->is_AddP(), "should be base + off");
-  Node* base_src = src->in(AddPNode::Base);
-  Node* base_dest = dest->in(AddPNode::Base);
+  const Type* src_type = phase->type(base_src);
 
   MergeMemNode* mem = MergeMemNode::make(in_mem);
 
@@ -208,7 +203,6 @@ Node* ArrayCopyNode::try_clone_instance(PhaseGVN *phase, bool can_reshape, int c
   BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
   for (int i = 0; i < count; i++) {
     ciField* field = ik->nonstatic_field_at(i);
-    int fieldidx = phase->C->alias_type(field)->index();
     const TypePtr* adr_type = phase->C->alias_type(field)->adr_type();
     Node* off = phase->MakeConX(field->offset());
     Node* next_src = phase->transform(new AddPNode(base_src,base_src,off));
@@ -247,16 +241,17 @@ bool ArrayCopyNode::prepare_array_copy(PhaseGVN *phase, bool can_reshape,
                                        BasicType& copy_type,
                                        const Type*& value_type,
                                        bool& disjoint_bases) {
-  Node* src = in(ArrayCopyNode::Src);
-  Node* dest = in(ArrayCopyNode::Dest);
-  const Type* src_type = phase->type(src);
+  base_src = in(ArrayCopyNode::Src);
+  base_dest = in(ArrayCopyNode::Dest);
+  const Type* src_type = phase->type(base_src);
   const TypeAryPtr* ary_src = src_type->isa_aryptr();
 
+  Node* src_offset = in(ArrayCopyNode::SrcPos);
+  Node* dest_offset = in(ArrayCopyNode::DestPos);
+
   if (is_arraycopy() || is_copyofrange() || is_copyof()) {
-    const Type* dest_type = phase->type(dest);
+    const Type* dest_type = phase->type(base_dest);
     const TypeAryPtr* ary_dest = dest_type->isa_aryptr();
-    Node* src_offset = in(ArrayCopyNode::SrcPos);
-    Node* dest_offset = in(ArrayCopyNode::DestPos);
 
     // newly allocated object is guaranteed to not overlap with source object
     disjoint_bases = is_alloc_tightly_coupled();
@@ -286,63 +281,56 @@ bool ArrayCopyNode::prepare_array_copy(PhaseGVN *phase, bool can_reshape,
 
     value_type = ary_src->elem();
 
-    base_src = src;
-    base_dest = dest;
-
     uint shift  = exact_log2(type2aelembytes(dest_elem));
     uint header = arrayOopDesc::base_offset_in_bytes(dest_elem);
 
-    adr_src = src;
-    adr_dest = dest;
-
     src_offset = Compile::conv_I2X_index(phase, src_offset, ary_src->size());
     dest_offset = Compile::conv_I2X_index(phase, dest_offset, ary_dest->size());
     if (src_offset->is_top() || dest_offset->is_top()) {
       // Offset is out of bounds (the ArrayCopyNode will be removed)
       return false;
     }
 
-    Node* src_scale = phase->transform(new LShiftXNode(src_offset, phase->intcon(shift)));
+    Node* src_scale  = phase->transform(new LShiftXNode(src_offset, phase->intcon(shift)));
     Node* dest_scale = phase->transform(new LShiftXNode(dest_offset, phase->intcon(shift)));
 
-    adr_src = phase->transform(new AddPNode(base_src, adr_src, src_scale));
-    adr_dest = phase->transform(new AddPNode(base_dest, adr_dest, dest_scale));
+    adr_src          = phase->transform(new AddPNode(base_src, base_src, src_scale));
+    adr_dest         = phase->transform(new AddPNode(base_dest, base_dest, dest_scale));
 
-    adr_src = new AddPNode(base_src, adr_src, phase->MakeConX(header));
-    adr_dest = new AddPNode(base_dest, adr_dest, phase->MakeConX(header));
-
-    adr_src = phase->transform(adr_src);
-    adr_dest = phase->transform(adr_dest);
+    adr_src          = phase->transform(new AddPNode(base_src, adr_src, phase->MakeConX(header)));
+    adr_dest         = phase->transform(new AddPNode(base_dest, adr_dest, phase->MakeConX(header)));
 
     copy_type = dest_elem;
   } else {
     assert(ary_src != NULL, "should be a clone");
     assert(is_clonebasic(), "should be");
 
     disjoint_bases = true;
-    assert(src->is_AddP(), "should be base + off");
-    assert(dest->is_AddP(), "should be base + off");
-    adr_src = src;
-    base_src = src->in(AddPNode::Base);
-    adr_dest = dest;
-    base_dest = dest->in(AddPNode::Base);
-
-    assert(phase->type(src->in(AddPNode::Offset))->is_intptr_t()->get_con() == phase->type(dest->in(AddPNode::Offset))->is_intptr_t()->get_con(), "same start offset?");
+
+    adr_src  = phase->transform(new AddPNode(base_src, base_src, src_offset));
+    adr_dest = phase->transform(new AddPNode(base_dest, base_dest, dest_offset));
+
     BasicType elem = ary_src->klass()->as_array_klass()->element_type()->basic_type();
-    if (is_reference_type(elem))  elem = T_OBJECT;
+    if (is_reference_type(elem)) {
+      elem = T_OBJECT;
+    }
 
     BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
     if (bs->array_copy_requires_gc_barriers(true, elem, true, BarrierSetC2::Optimization)) {
       return false;
     }
 
-    int diff = arrayOopDesc::base_offset_in_bytes(elem) - phase->type(src->in(AddPNode::Offset))->is_intptr_t()->get_con();
+    // The address is offseted to an aligned address where a raw copy would start.
+    // If the clone copy is decomposed into load-stores - the address is adjusted to
+    // point at where the array starts.
+    const Type* toff = phase->type(src_offset);
+    int offset = toff->isa_long() ? (int) toff->is_long()->get_con() : (int) toff->is_int()->get_con();
+    int diff = arrayOopDesc::base_offset_in_bytes(elem) - offset;
     assert(diff >= 0, "clone should not start after 1st array element");
     if (diff > 0) {
       adr_src = phase->transform(new AddPNode(base_src, adr_src, phase->MakeConX(diff)));
       adr_dest = phase->transform(new AddPNode(base_dest, adr_dest, phase->MakeConX(diff)));
     }
-
     copy_type = elem;
     value_type = ary_src->elem();
   }
@@ -535,8 +523,8 @@ Node *ArrayCopyNode::Ideal(PhaseGVN *phase, bool can_reshape) {
          in(ArrayCopyNode::Src) != NULL &&
          in(ArrayCopyNode::Dest) != NULL &&
          in(ArrayCopyNode::Length) != NULL &&
-         ((in(ArrayCopyNode::SrcPos) != NULL && in(ArrayCopyNode::DestPos) != NULL) ||
-          is_clonebasic()), "broken inputs");
+         in(ArrayCopyNode::SrcPos) != NULL &&
+         in(ArrayCopyNode::DestPos) != NULL, "broken inputs");
 
   if (in(TypeFunc::Control)->is_top() ||
       in(TypeFunc::Memory)->is_top() ||
@@ -582,7 +570,6 @@ Node *ArrayCopyNode::Ideal(PhaseGVN *phase, bool can_reshape) {
     in_mem = MergeMemNode::make(in_mem);
   }
 
-
   if (can_reshape) {
     assert(!phase->is_IterGVN()->delay_transform(), "cannot delay transforms");
     phase->is_IterGVN()->set_delay_transform(true);

test/hotspot/jtreg/compiler/arguments/TestStressReflectiveCode.java

@@ -35,8 +35,25 @@ public class TestStressReflectiveCode {
 
     public static void main(String[] args) {
         VALUES.clone();
+        VALUES2.clone();
     }
 
-    private static final int[] VALUES = new int[]{3, 4, 5};
+    public static class Payload implements Cloneable {
+        int i1;
+        int i2;
+        int i3;
+        int i4;
+
+        public Object clone() {
+          try {
+            return super.clone();
+          } catch (CloneNotSupportedException e) {
+          }
+          return null;
+        }
+    }
+
+    private static final int[]   VALUES = new int[]{3, 4, 5};
+    private static final Payload VALUES2 = new Payload();
 }