# HG changeset patch # User Martin Doerr # Date 1377681763 -7200 # Node ID 2c2d3b25ecfaee78430b2870f4759b66867177f7 # Parent 73921c720b943ee44be1e883c8383baf512a5c20 8023597: Optimize G1 barriers code for unsafe load_store Summary: Avoid loading old values in G1 pre-barriers for inlined unsafe load_store nodes. diff --git a/src/share/vm/opto/graphKit.cpp b/src/share/vm/opto/graphKit.cpp --- a/src/share/vm/opto/graphKit.cpp +++ b/src/share/vm/opto/graphKit.cpp @@ -1501,6 +1501,25 @@ } } +bool GraphKit::can_move_pre_barrier() const { + BarrierSet* bs = Universe::heap()->barrier_set(); + switch (bs->kind()) { + case BarrierSet::G1SATBCT: + case BarrierSet::G1SATBCTLogging: + return true; // Can move it if no safepoint + + case BarrierSet::CardTableModRef: + case BarrierSet::CardTableExtension: + case BarrierSet::ModRef: + return true; // There is no pre-barrier + + case BarrierSet::Other: + default : + ShouldNotReachHere(); + } + return false; +} + void GraphKit::post_barrier(Node* ctl, Node* store, Node* obj, @@ -3551,6 +3570,8 @@ } else { // In this case both val_type and alias_idx are unused. assert(pre_val != NULL, "must be loaded already"); + // Nothing to be done if pre_val is null. + if (pre_val->bottom_type() == TypePtr::NULL_PTR) return; assert(pre_val->bottom_type()->basic_type() == T_OBJECT, "or we shouldn't be here"); } assert(bt == T_OBJECT, "or we shouldn't be here"); diff --git a/src/share/vm/opto/graphKit.hpp b/src/share/vm/opto/graphKit.hpp --- a/src/share/vm/opto/graphKit.hpp +++ b/src/share/vm/opto/graphKit.hpp @@ -695,6 +695,9 @@ void write_barrier_post(Node *store, Node* obj, Node* adr, uint adr_idx, Node* val, bool use_precise); + // allow reordering of pre-barrier with oop store and/or post-barrier + bool can_move_pre_barrier() const; + // G1 pre/post barriers void g1_write_barrier_pre(bool do_load, Node* obj, diff --git a/src/share/vm/opto/library_call.cpp b/src/share/vm/opto/library_call.cpp --- a/src/share/vm/opto/library_call.cpp +++ b/src/share/vm/opto/library_call.cpp @@ -2756,10 +2756,28 @@ newval = _gvn.makecon(TypePtr::NULL_PTR); // Reference stores need a store barrier. - pre_barrier(true /* do_load*/, - control(), base, adr, alias_idx, newval, value_type->make_oopptr(), - NULL /* pre_val*/, - T_OBJECT); + if (kind == LS_xchg) { + // If pre-barrier must execute before the oop store, old value will require do_load here. + if (!can_move_pre_barrier()) { + pre_barrier(true /* do_load*/, + control(), base, adr, alias_idx, newval, value_type->make_oopptr(), + NULL /* pre_val*/, + T_OBJECT); + } // Else: see below. + } else if (kind == LS_cmpxchg) { + // Same as for newval above: + if (_gvn.type(oldval) == TypePtr::NULL_PTR) { + oldval = _gvn.makecon(TypePtr::NULL_PTR); + } + // The only known value which might get overwritten is oldval. + pre_barrier(false /* do_load */, + control(), NULL, NULL, max_juint, NULL, NULL, + oldval /* pre_val */, + T_OBJECT); + } else { + ShouldNotReachHere(); + } + #ifdef _LP64 if (adr->bottom_type()->is_ptr_to_narrowoop()) { Node *newval_enc = _gvn.transform(new (C) EncodePNode(newval, newval->bottom_type()->make_narrowoop())); @@ -2795,16 +2813,27 @@ Node* proj = _gvn.transform(new (C) SCMemProjNode(load_store)); set_memory(proj, alias_idx); + if (type == T_OBJECT && kind == LS_xchg) { +#ifdef _LP64 + if (adr->bottom_type()->is_ptr_to_narrowoop()) { + load_store = _gvn.transform(new (C) DecodeNNode(load_store, load_store->get_ptr_type())); + } +#endif + if (can_move_pre_barrier()) { + // Don't need to load pre_val. The old value is returned by load_store. + // The pre_barrier can execute after the xchg as long as no safepoint + // gets inserted between them. + pre_barrier(false /* do_load */, + control(), NULL, NULL, max_juint, NULL, NULL, + load_store /* pre_val */, + T_OBJECT); + } + } + // Add the trailing membar surrounding the access insert_mem_bar(Op_MemBarCPUOrder); insert_mem_bar(Op_MemBarAcquire); -#ifdef _LP64 - if (type == T_OBJECT && adr->bottom_type()->is_ptr_to_narrowoop() && kind == LS_xchg) { - load_store = _gvn.transform(new (C) DecodeNNode(load_store, load_store->get_ptr_type())); - } -#endif - assert(type2size[load_store->bottom_type()->basic_type()] == type2size[rtype], "result type should match"); set_result(load_store); return true;