# HG changeset patch # User neliasso # Date 1575023185 -3600 # Node ID 5e8f9713e34303fe0a966000cf8044c83de54e01 # Parent ae2eb76c486d27d37065c62902d530833602c8b8 8234520: ZGC: C2: Oop instance cloning causing skipped compiles Reviewed-by: pliden, vlivanov diff -r ae2eb76c486d -r 5e8f9713e343 src/hotspot/share/gc/shared/c2/barrierSetC2.cpp --- 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; diff -r ae2eb76c486d -r 5e8f9713e343 src/hotspot/share/gc/shared/c2/barrierSetC2.hpp --- 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 diff -r ae2eb76c486d -r 5e8f9713e343 src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp --- 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); diff -r ae2eb76c486d -r 5e8f9713e343 src/hotspot/share/gc/z/zBarrierSetRuntime.cpp --- 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 diff -r ae2eb76c486d -r 5e8f9713e343 src/hotspot/share/gc/z/zBarrierSetRuntime.hpp --- 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); diff -r ae2eb76c486d -r 5e8f9713e343 src/hotspot/share/opto/arraycopynode.hpp --- 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; }