# HG changeset patch # User thartmann # Date 1464850012 -7200 # Node ID fb4492288b01a7e2aeb264b8c7605379e1db8b27 # Parent bd5fe208734e02fdb091b2a7329db894c4b80ef5 8156760: VM crashes if -XX:-ReduceInitialCardMarks is set Summary: Fixed several compiler crashes with disabled ReduceInitialCardMarks. Reviewed-by: roland, minqi, dlong, tschatzl, kvn diff -r bd5fe208734e -r fb4492288b01 hotspot/src/share/vm/gc/g1/g1CollectedHeap.inline.hpp --- a/hotspot/src/share/vm/gc/g1/g1CollectedHeap.inline.hpp Wed Jun 01 16:36:44 2016 +0200 +++ b/hotspot/src/share/vm/gc/g1/g1CollectedHeap.inline.hpp Thu Jun 02 08:46:52 2016 +0200 @@ -109,7 +109,7 @@ _old_set.remove(hr); } -// It dirties the cards that cover the block so that so that the post +// It dirties the cards that cover the block so that the post // write barrier never queues anything when updating objects on this // block. It is assumed (and in fact we assert) that the block // belongs to a young region. diff -r bd5fe208734e -r fb4492288b01 hotspot/src/share/vm/gc/shared/collectedHeap.cpp --- a/hotspot/src/share/vm/gc/shared/collectedHeap.cpp Wed Jun 01 16:36:44 2016 +0200 +++ b/hotspot/src/share/vm/gc/shared/collectedHeap.cpp Thu Jun 02 08:46:52 2016 +0200 @@ -386,7 +386,7 @@ // initialized by this point, a fact that we assert when doing the // card-mark.) // (c) G1CollectedHeap(G1) uses two kinds of write barriers. When a -// G1 concurrent marking is in progress an SATB (pre-write-)barrier is +// G1 concurrent marking is in progress an SATB (pre-write-)barrier // is used to remember the pre-value of any store. Initializing // stores will not need this barrier, so we need not worry about // compensating for the missing pre-barrier here. Turning now diff -r bd5fe208734e -r fb4492288b01 hotspot/src/share/vm/opto/arraycopynode.cpp --- a/hotspot/src/share/vm/opto/arraycopynode.cpp Wed Jun 01 16:36:44 2016 +0200 +++ b/hotspot/src/share/vm/opto/arraycopynode.cpp Thu Jun 02 08:46:52 2016 +0200 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -204,7 +204,8 @@ } if (!finish_transform(phase, can_reshape, ctl, mem)) { - return NULL; + // Return NodeSentinel to indicate that the transform failed + return NodeSentinel; } return mem; @@ -222,6 +223,7 @@ Node* dest = in(ArrayCopyNode::Dest); const Type* src_type = phase->type(src); const TypeAryPtr* ary_src = src_type->isa_aryptr(); + assert(ary_src != NULL, "should be an array copy/clone"); if (is_arraycopy() || is_copyofrange() || is_copyof()) { const Type* dest_type = phase->type(dest); @@ -520,7 +522,7 @@ Node* mem = try_clone_instance(phase, can_reshape, count); if (mem != NULL) { - return mem; + return (mem == NodeSentinel) ? NULL : mem; } Node* adr_src = NULL; @@ -627,31 +629,37 @@ return CallNode::may_modify_arraycopy_helper(dest_t, t_oop, phase); } -bool ArrayCopyNode::may_modify_helper(const TypeOopPtr *t_oop, Node* n, PhaseTransform *phase) { +bool ArrayCopyNode::may_modify_helper(const TypeOopPtr *t_oop, Node* n, PhaseTransform *phase, ArrayCopyNode*& ac) { if (n->is_Proj()) { n = n->in(0); if (n->is_Call() && n->as_Call()->may_modify(t_oop, phase)) { + if (n->isa_ArrayCopy() != NULL) { + ac = n->as_ArrayCopy(); + } return true; } } return false; } -bool ArrayCopyNode::may_modify(const TypeOopPtr *t_oop, MemBarNode* mb, PhaseTransform *phase) { +bool ArrayCopyNode::may_modify(const TypeOopPtr *t_oop, MemBarNode* mb, PhaseTransform *phase, ArrayCopyNode*& ac) { Node* mem = mb->in(TypeFunc::Memory); if (mem->is_MergeMem()) { Node* n = mem->as_MergeMem()->memory_at(Compile::AliasIdxRaw); - if (may_modify_helper(t_oop, n, phase)) { + if (may_modify_helper(t_oop, n, phase, ac)) { return true; } else if (n->is_Phi()) { for (uint i = 1; i < n->req(); i++) { if (n->in(i) != NULL) { - if (may_modify_helper(t_oop, n->in(i), phase)) { + if (may_modify_helper(t_oop, n->in(i), phase, ac)) { return true; } } } + } else if (n->Opcode() == Op_StoreCM) { + // Ignore card mark stores + return may_modify_helper(t_oop, n->in(MemNode::Memory), phase, ac); } } diff -r bd5fe208734e -r fb4492288b01 hotspot/src/share/vm/opto/arraycopynode.hpp --- a/hotspot/src/share/vm/opto/arraycopynode.hpp Wed Jun 01 16:36:44 2016 +0200 +++ b/hotspot/src/share/vm/opto/arraycopynode.hpp Thu Jun 02 08:46:52 2016 +0200 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -107,7 +107,7 @@ BasicType copy_type, const Type* value_type, int count); bool finish_transform(PhaseGVN *phase, bool can_reshape, Node* ctl, Node *mem); - static bool may_modify_helper(const TypeOopPtr *t_oop, Node* n, PhaseTransform *phase); + static bool may_modify_helper(const TypeOopPtr *t_oop, Node* n, PhaseTransform *phase, ArrayCopyNode*& ac); public: @@ -162,7 +162,7 @@ bool is_alloc_tightly_coupled() const { return _alloc_tightly_coupled; } - static bool may_modify(const TypeOopPtr *t_oop, MemBarNode* mb, PhaseTransform *phase); + static bool may_modify(const TypeOopPtr *t_oop, MemBarNode* mb, PhaseTransform *phase, ArrayCopyNode*& ac); bool modifies(intptr_t offset_lo, intptr_t offset_hi, PhaseTransform* phase, bool must_modify); #ifndef PRODUCT diff -r bd5fe208734e -r fb4492288b01 hotspot/src/share/vm/opto/graphKit.cpp --- a/hotspot/src/share/vm/opto/graphKit.cpp Wed Jun 01 16:36:44 2016 +0200 +++ b/hotspot/src/share/vm/opto/graphKit.cpp Thu Jun 02 08:46:52 2016 +0200 @@ -4306,8 +4306,15 @@ } __ end_if(); } __ end_if(); } else { - // Object.clone() instrinsic uses this path. - g1_mark_card(ideal, card_adr, oop_store, alias_idx, index, index_adr, buffer, tf); + // The Object.clone() intrinsic uses this path if !ReduceInitialCardMarks. + // We don't need a barrier here if the destination is a newly allocated object + // in Eden. Otherwise, GC verification breaks because we assume that cards in Eden + // are set to 'g1_young_gen' (see G1SATBCardTableModRefBS::verify_g1_young_region()). + assert(!use_ReduceInitialCardMarks(), "can only happen with card marking"); + Node* card_val = __ load(__ ctrl(), card_adr, TypeInt::INT, T_BYTE, Compile::AliasIdxRaw); + __ if_then(card_val, BoolTest::ne, young_card); { + g1_mark_card(ideal, card_adr, oop_store, alias_idx, index, index_adr, buffer, tf); + } __ end_if(); } // Final sync IdealKit and GraphKit. diff -r bd5fe208734e -r fb4492288b01 hotspot/src/share/vm/opto/macro.cpp --- a/hotspot/src/share/vm/opto/macro.cpp Wed Jun 01 16:36:44 2016 +0200 +++ b/hotspot/src/share/vm/opto/macro.cpp Thu Jun 02 08:46:52 2016 +0200 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -32,6 +32,7 @@ #include "opto/cfgnode.hpp" #include "opto/compile.hpp" #include "opto/convertnode.hpp" +#include "opto/graphKit.hpp" #include "opto/locknode.hpp" #include "opto/loopnode.hpp" #include "opto/macro.hpp" @@ -263,41 +264,58 @@ // checks if the store done to a different from the value's region. // And replace Cmp with #0 (false) to collapse G1 post barrier. Node* xorx = p2x->find_out_with(Op_XorX); - assert(xorx != NULL, "missing G1 post barrier"); - Node* shift = xorx->unique_out(); - Node* cmpx = shift->unique_out(); - assert(cmpx->is_Cmp() && cmpx->unique_out()->is_Bool() && - cmpx->unique_out()->as_Bool()->_test._test == BoolTest::ne, - "missing region check in G1 post barrier"); - _igvn.replace_node(cmpx, makecon(TypeInt::CC_EQ)); + if (xorx != NULL) { + Node* shift = xorx->unique_out(); + Node* cmpx = shift->unique_out(); + assert(cmpx->is_Cmp() && cmpx->unique_out()->is_Bool() && + cmpx->unique_out()->as_Bool()->_test._test == BoolTest::ne, + "missing region check in G1 post barrier"); + _igvn.replace_node(cmpx, makecon(TypeInt::CC_EQ)); - // Remove G1 pre barrier. + // Remove G1 pre barrier. - // Search "if (marking != 0)" check and set it to "false". - // 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) { - int ind = 1; - if (!this_region->in(ind)->is_IfFalse()) { - ind = 2; - } - if (this_region->in(ind)->is_IfFalse()) { - Node* bol = this_region->in(ind)->in(0)->in(1); - assert(bol->is_Bool(), ""); - cmpx = bol->in(1); - if (bol->as_Bool()->_test._test == BoolTest::ne && - cmpx->is_Cmp() && cmpx->in(2) == intcon(0) && - cmpx->in(1)->is_Load()) { - Node* adr = cmpx->in(1)->as_Load()->in(MemNode::Address); - const int marking_offset = in_bytes(JavaThread::satb_mark_queue_offset() + - SATBMarkQueue::byte_offset_of_active()); - if (adr->is_AddP() && adr->in(AddPNode::Base) == top() && - adr->in(AddPNode::Address)->Opcode() == Op_ThreadLocal && - adr->in(AddPNode::Offset) == MakeConX(marking_offset)) { - _igvn.replace_node(cmpx, makecon(TypeInt::CC_EQ)); + // Search "if (marking != 0)" check and set it to "false". + // 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) { + int ind = 1; + if (!this_region->in(ind)->is_IfFalse()) { + ind = 2; + } + if (this_region->in(ind)->is_IfFalse()) { + Node* bol = this_region->in(ind)->in(0)->in(1); + assert(bol->is_Bool(), ""); + cmpx = bol->in(1); + if (bol->as_Bool()->_test._test == BoolTest::ne && + cmpx->is_Cmp() && cmpx->in(2) == intcon(0) && + cmpx->in(1)->is_Load()) { + Node* adr = cmpx->in(1)->as_Load()->in(MemNode::Address); + const int marking_offset = in_bytes(JavaThread::satb_mark_queue_offset() + + SATBMarkQueue::byte_offset_of_active()); + if (adr->is_AddP() && adr->in(AddPNode::Base) == top() && + adr->in(AddPNode::Address)->Opcode() == Op_ThreadLocal && + adr->in(AddPNode::Offset) == MakeConX(marking_offset)) { + _igvn.replace_node(cmpx, makecon(TypeInt::CC_EQ)); + } } } } + } else { + assert(!GraphKit::use_ReduceInitialCardMarks(), "can only happen with card marking"); + // This is a G1 post barrier emitted by the Object.clone() intrinsic. + // Search for the CastP2X->URShiftX->AddP->LoadB->Cmp path which checks if the card + // is marked as young_gen and replace the Cmp with 0 (false) to collapse the barrier. + Node* shift = p2x->find_out_with(Op_URShiftX); + assert(shift != NULL, "missing G1 post barrier"); + Node* addp = shift->unique_out(); + Node* load = addp->find_out_with(Op_LoadB); + assert(load != NULL, "missing G1 post barrier"); + Node* cmpx = load->unique_out(); + assert(cmpx->is_Cmp() && cmpx->unique_out()->is_Bool() && + cmpx->unique_out()->as_Bool()->_test._test == BoolTest::ne, + "missing card value check in G1 post barrier"); + _igvn.replace_node(cmpx, makecon(TypeInt::CC_EQ)); + // There is no G1 pre barrier in this case } // Now CastP2X can be removed since it is used only on dead path // which currently still alive until igvn optimize it. @@ -326,17 +344,15 @@ CallNode *call = in->as_Call(); if (call->may_modify(tinst, phase)) { assert(call->is_ArrayCopy(), "ArrayCopy is the only call node that doesn't make allocation escape"); - if (call->as_ArrayCopy()->modifies(offset, offset, phase, false)) { return in; } } mem = in->in(TypeFunc::Memory); } else if (in->is_MemBar()) { - if (ArrayCopyNode::may_modify(tinst, in->as_MemBar(), phase)) { - assert(in->in(0)->is_Proj() && in->in(0)->in(0)->is_ArrayCopy(), "should be arraycopy"); - ArrayCopyNode* ac = in->in(0)->in(0)->as_ArrayCopy(); - assert(ac->is_clonebasic(), "Only basic clone is a non escaping clone"); + ArrayCopyNode* ac = NULL; + if (ArrayCopyNode::may_modify(tinst, in->as_MemBar(), phase, ac)) { + assert(ac != NULL && ac->is_clonebasic(), "Only basic clone is a non escaping clone"); return ac; } mem = in->in(TypeFunc::Memory); diff -r bd5fe208734e -r fb4492288b01 hotspot/src/share/vm/opto/memnode.cpp --- a/hotspot/src/share/vm/opto/memnode.cpp Wed Jun 01 16:36:44 2016 +0200 +++ b/hotspot/src/share/vm/opto/memnode.cpp Thu Jun 02 08:46:52 2016 +0200 @@ -160,7 +160,8 @@ } } } else if (proj_in->is_MemBar()) { - if (ArrayCopyNode::may_modify(t_oop, proj_in->as_MemBar(), phase)) { + ArrayCopyNode* ac = NULL; + if (ArrayCopyNode::may_modify(t_oop, proj_in->as_MemBar(), phase, ac)) { break; } result = proj_in->in(TypeFunc::Memory); @@ -657,7 +658,8 @@ continue; // (a) advance through independent call memory } } else if (mem->is_Proj() && mem->in(0)->is_MemBar()) { - if (ArrayCopyNode::may_modify(addr_t, mem->in(0)->as_MemBar(), phase)) { + ArrayCopyNode* ac = NULL; + if (ArrayCopyNode::may_modify(addr_t, mem->in(0)->as_MemBar(), phase, ac)) { break; } mem = mem->in(0)->in(TypeFunc::Memory); diff -r bd5fe208734e -r fb4492288b01 hotspot/test/compiler/arraycopy/TestEliminatedArrayCopyDeopt.java --- a/hotspot/test/compiler/arraycopy/TestEliminatedArrayCopyDeopt.java Wed Jun 01 16:36:44 2016 +0200 +++ b/hotspot/test/compiler/arraycopy/TestEliminatedArrayCopyDeopt.java Thu Jun 02 08:46:52 2016 +0200 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,10 +23,10 @@ /* * @test - * @bug 8130847 + * @bug 8130847 8156760 * @summary Eliminated instance/array written to by an array copy variant must be correctly initialized when reallocated at a deopt * @run main/othervm -XX:-BackgroundCompilation -XX:-UseOnStackReplacement TestEliminatedArrayCopyDeopt - * + * @run main/othervm -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:+IgnoreUnrecognizedVMOptions -XX:-ReduceInitialCardMarks TestEliminatedArrayCopyDeopt */ // Test that if an ArrayCopy node is eliminated because it doesn't diff -r bd5fe208734e -r fb4492288b01 hotspot/test/compiler/arraycopy/TestInstanceCloneAsLoadsStores.java --- a/hotspot/test/compiler/arraycopy/TestInstanceCloneAsLoadsStores.java Wed Jun 01 16:36:44 2016 +0200 +++ b/hotspot/test/compiler/arraycopy/TestInstanceCloneAsLoadsStores.java Thu Jun 02 08:46:52 2016 +0200 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,12 +23,12 @@ /* * @test - * @bug 6700100 + * @bug 6700100 8156760 * @summary small instance clone as loads/stores * @compile TestInstanceCloneAsLoadsStores.java TestInstanceCloneUtils.java * @run main/othervm -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:CompileCommand=dontinline,TestInstanceCloneAsLoadsStores::m* TestInstanceCloneAsLoadsStores * @run main/othervm -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:CompileCommand=dontinline,TestInstanceCloneAsLoadsStores::m* -XX:+IgnoreUnrecognizedVMOptions -XX:+StressArrayCopyMacroNode TestInstanceCloneAsLoadsStores - * + * @run main/othervm -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:CompileCommand=dontinline,TestInstanceCloneAsLoadsStores::m* -XX:+IgnoreUnrecognizedVMOptions -XX:-ReduceInitialCardMarks TestInstanceCloneAsLoadsStores */ public class TestInstanceCloneAsLoadsStores extends TestInstanceCloneUtils {