From 6de77a177550f94b6b463097cc2f64c042897c4a Mon Sep 17 00:00:00 2001 From: kvn Date: Tue, 7 Feb 2012 16:33:43 -0800 Subject: [PATCH] 7143491: G1 C2 CTW: assert(p2x->outcnt() == 2) failed: expects 2 users: Xor and URShift nodes Summary: Adjust the assert and code in eliminate_card_mark() method for case when stored value is NULL. Reviewed-by: iveresov, never --- src/share/vm/opto/graphKit.cpp | 5 +++++ src/share/vm/opto/library_call.cpp | 8 +++++++- src/share/vm/opto/macro.cpp | 15 +++++++++++---- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/share/vm/opto/graphKit.cpp b/src/share/vm/opto/graphKit.cpp index 565896079..61d922760 100644 --- a/src/share/vm/opto/graphKit.cpp +++ b/src/share/vm/opto/graphKit.cpp @@ -1522,6 +1522,11 @@ Node* GraphKit::store_oop(Node* ctl, const TypeOopPtr* val_type, BasicType bt, bool use_precise) { + // Transformation of a value which could be NULL pointer (CastPP #NULL) + // could be delayed during Parse (for example, in adjust_map_after_if()). + // Execute transformation here to avoid barrier generation in such case. + if (_gvn.type(val) == TypePtr::NULL_PTR) + val = _gvn.makecon(TypePtr::NULL_PTR); set_control(ctl); if (stopped()) return top(); // Dead path ? diff --git a/src/share/vm/opto/library_call.cpp b/src/share/vm/opto/library_call.cpp index ebc3a2dd9..57730de6b 100644 --- a/src/share/vm/opto/library_call.cpp +++ b/src/share/vm/opto/library_call.cpp @@ -2678,7 +2678,13 @@ bool LibraryCallKit::inline_unsafe_CAS(BasicType type) { cas = _gvn.transform(new (C, 5) CompareAndSwapLNode(control(), mem, adr, newval, oldval)); break; case T_OBJECT: - // reference stores need a store barrier. + // Transformation of a value which could be NULL pointer (CastPP #NULL) + // could be delayed during Parse (for example, in adjust_map_after_if()). + // Execute transformation here to avoid barrier generation in such case. + if (_gvn.type(newval) == TypePtr::NULL_PTR) + newval = _gvn.makecon(TypePtr::NULL_PTR); + + // Reference stores need a store barrier. // (They don't if CAS fails, but it isn't worth checking.) pre_barrier(true /* do_load*/, control(), base, adr, alias_idx, newval, value_type->make_oopptr(), diff --git a/src/share/vm/opto/macro.cpp b/src/share/vm/opto/macro.cpp index adcf8e48f..11416ac47 100644 --- a/src/share/vm/opto/macro.cpp +++ b/src/share/vm/opto/macro.cpp @@ -234,11 +234,20 @@ void PhaseMacroExpand::eliminate_card_mark(Node* p2x) { } } else { // G1 pre/post barriers - assert(p2x->outcnt() == 2, "expects 2 users: Xor and URShift nodes"); + assert(p2x->outcnt() <= 2, "expects 1 or 2 users: Xor and URShift nodes"); // It could be only one user, URShift node, in Object.clone() instrinsic // but the new allocation is passed to arraycopy stub and it could not // be scalar replaced. So we don't check the case. + // An other case of only one user (Xor) is when the value check for NULL + // in G1 post barrier is folded after CCP so the code which used URShift + // is removed. + + // Take Region node before eliminating post barrier since it also + // eliminates CastP2X node when it has only one user. + Node* this_region = p2x->in(0); + assert(this_region != NULL, ""); + // Remove G1 post barrier. // Search for CastP2X->Xor->URShift->Cmp path which @@ -263,8 +272,6 @@ void PhaseMacroExpand::eliminate_card_mark(Node* p2x) { // Remove G1 pre barrier. // Search "if (marking != 0)" check and set it to "false". - Node* this_region = p2x->in(0); - assert(this_region != NULL, ""); // There is no G1 pre barrier if previous stored value is NULL // (for example, after initialization). if (this_region->is_Region() && this_region->req() == 3) { @@ -292,7 +299,7 @@ void PhaseMacroExpand::eliminate_card_mark(Node* p2x) { } // Now CastP2X can be removed since it is used only on dead path // which currently still alive until igvn optimize it. - assert(p2x->unique_out()->Opcode() == Op_URShiftX, ""); + assert(p2x->outcnt() == 0 || p2x->unique_out()->Opcode() == Op_URShiftX, ""); _igvn.replace_node(p2x, top()); } } -- GitLab