stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8156760: VM crashes if -XX:-ReduceInitialCardMarks is set

Browse Source 
Fixed several compiler crashes with disabled ReduceInitialCardMarks.

Reviewed-by: roland, minqi, dlong, tschatzl, kvn
This commit is contained in:
Tobias Hartmann 2016-06-02 08:46:52 +02:00
parent e4102fbe67
commit 35f9db149b
9 changed files with 91 additions and 58 deletions

2
hotspot/src/share/vm/gc/g1/g1CollectedHeap.inline.hpp

@ -109,7 +109,7 @@ inline void G1CollectedHeap::old_set_remove(HeapRegion* hr) {
_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.

2
hotspot/src/share/vm/gc/shared/collectedHeap.cpp

@ -386,7 +386,7 @@ size_t CollectedHeap::max_tlab_size() const {
// 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

22
hotspot/src/share/vm/opto/arraycopynode.cpp

@ -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 @@ Node* ArrayCopyNode::try_clone_instance(PhaseGVN *phase, bool can_reshape, int c
}
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 @@ bool ArrayCopyNode::prepare_array_copy(PhaseGVN *phase, bool can_reshape,
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 *ArrayCopyNode::Ideal(PhaseGVN *phase, bool can_reshape) {
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 @@ bool ArrayCopyNode::may_modify(const TypeOopPtr *t_oop, PhaseTransform *phase) {
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);
}
}

6
hotspot/src/share/vm/opto/arraycopynode.hpp

@ -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 @@ private:
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 @@ public:
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

11
hotspot/src/share/vm/opto/graphKit.cpp

@ -4306,8 +4306,15 @@ void GraphKit::g1_write_barrier_post(Node* oop_store,
} __ 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.

88
hotspot/src/share/vm/opto/macro.cpp

@ -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 @@ void PhaseMacroExpand::eliminate_card_mark(Node* p2x) {
// 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 @@ static Node *scan_mem_chain(Node *mem, int alias_idx, int offset, Node *start_me
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);

6
hotspot/src/share/vm/opto/memnode.cpp

@ -160,7 +160,8 @@ Node *MemNode::optimize_simple_memory_chain(Node *mchain, const TypeOopPtr *t_oo
}
}
} 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 @@ Node* MemNode::find_previous_store(PhaseTransform* phase) {
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);

6
hotspot/test/compiler/arraycopy/TestEliminatedArrayCopyDeopt.java

@ -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

6
hotspot/test/compiler/arraycopy/TestInstanceCloneAsLoadsStores.java

@ -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 {