8023597: Optimize G1 barriers code for unsafe load_store
Summary: Avoid loading old values in G1 pre-barriers for inlined unsafe load_store nodes.
Reviewed-by: kvn, tonyp
Contributed-by: Martin Doerr <martin.doerr@sap.com>
--- a/hotspot/src/share/vm/opto/graphKit.cpp Tue Aug 27 21:16:54 2013 +0200
+++ b/hotspot/src/share/vm/opto/graphKit.cpp Wed Aug 28 11:22:43 2013 +0200
@@ -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");
--- a/hotspot/src/share/vm/opto/graphKit.hpp Tue Aug 27 21:16:54 2013 +0200
+++ b/hotspot/src/share/vm/opto/graphKit.hpp Wed Aug 28 11:22:43 2013 +0200
@@ -695,6 +695,10 @@
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.
+ // Used for load_store operations which loads old value.
+ bool can_move_pre_barrier() const;
+
// G1 pre/post barriers
void g1_write_barrier_pre(bool do_load,
Node* obj,
--- a/hotspot/src/share/vm/opto/library_call.cpp Tue Aug 27 21:16:54 2013 +0200
+++ b/hotspot/src/share/vm/opto/library_call.cpp Wed Aug 28 11:22:43 2013 +0200
@@ -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 move pre_barrier to use load_store value, 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;