6877254: Server vm crashes with no branches off of store slice" when run with CMS and UseSuperWord(default)
Summary: design StoreCMNode::Ideal to promote its oopStore input if the input is a MergeMem node
Reviewed-by: kvn, never
--- a/hotspot/src/share/vm/opto/graphKit.cpp Thu Sep 10 18:18:06 2009 -0700
+++ b/hotspot/src/share/vm/opto/graphKit.cpp Mon Sep 14 09:49:54 2009 -0700
@@ -1450,7 +1450,7 @@
case BarrierSet::CardTableModRef:
case BarrierSet::CardTableExtension:
- write_barrier_post(store, obj, adr, val, use_precise);
+ write_barrier_post(store, obj, adr, adr_idx, val, use_precise);
break;
case BarrierSet::ModRef:
@@ -3165,6 +3165,7 @@
void GraphKit::write_barrier_post(Node* oop_store,
Node* obj,
Node* adr,
+ uint adr_idx,
Node* val,
bool use_precise) {
// No store check needed if we're storing a NULL or an old object
@@ -3214,7 +3215,7 @@
__ store(__ ctrl(), card_adr, zero, bt, adr_type);
} else {
// Specialized path for CM store barrier
- __ storeCM(__ ctrl(), card_adr, zero, oop_store, bt, adr_type);
+ __ storeCM(__ ctrl(), card_adr, zero, oop_store, adr_idx, bt, adr_type);
}
// Final sync IdealKit and GraphKit.
@@ -3314,6 +3315,7 @@
void GraphKit::g1_mark_card(IdealKit& ideal,
Node* card_adr,
Node* oop_store,
+ uint oop_alias_idx,
Node* index,
Node* index_adr,
Node* buffer,
@@ -3323,7 +3325,7 @@
Node* no_base = __ top();
BasicType card_bt = T_BYTE;
// Smash zero into card. MUST BE ORDERED WRT TO STORE
- __ storeCM(__ ctrl(), card_adr, zero, oop_store, card_bt, Compile::AliasIdxRaw);
+ __ storeCM(__ ctrl(), card_adr, zero, oop_store, oop_alias_idx, card_bt, Compile::AliasIdxRaw);
// Now do the queue work
__ if_then(index, BoolTest::ne, zero); {
@@ -3435,13 +3437,13 @@
Node* card_val = __ load(__ ctrl(), card_adr, TypeInt::INT, T_BYTE, Compile::AliasIdxRaw);
__ if_then(card_val, BoolTest::ne, zero); {
- g1_mark_card(ideal, card_adr, oop_store, index, index_adr, buffer, tf);
+ g1_mark_card(ideal, card_adr, oop_store, alias_idx, index, index_adr, buffer, tf);
} __ end_if();
} __ end_if();
} __ end_if();
} else {
// Object.clone() instrinsic uses this path.
- g1_mark_card(ideal, card_adr, oop_store, index, index_adr, buffer, tf);
+ g1_mark_card(ideal, card_adr, oop_store, alias_idx, index, index_adr, buffer, tf);
}
// Final sync IdealKit and GraphKit.
--- a/hotspot/src/share/vm/opto/graphKit.hpp Thu Sep 10 18:18:06 2009 -0700
+++ b/hotspot/src/share/vm/opto/graphKit.hpp Mon Sep 14 09:49:54 2009 -0700
@@ -603,7 +603,8 @@
void sync_kit(IdealKit& ideal);
// vanilla/CMS post barrier
- void write_barrier_post(Node *store, Node* obj, Node* adr, Node* val, bool use_precise);
+ void write_barrier_post(Node *store, Node* obj,
+ Node* adr, uint adr_idx, Node* val, bool use_precise);
// G1 pre/post barriers
void g1_write_barrier_pre(Node* obj,
@@ -622,7 +623,8 @@
bool use_precise);
// Helper function for g1
private:
- void g1_mark_card(IdealKit& ideal, Node* card_adr, Node* store, Node* index, Node* index_adr,
+ void g1_mark_card(IdealKit& ideal, Node* card_adr, Node* store, uint oop_alias_idx,
+ Node* index, Node* index_adr,
Node* buffer, const TypeFunc* tf);
public:
--- a/hotspot/src/share/vm/opto/idealKit.cpp Thu Sep 10 18:18:06 2009 -0700
+++ b/hotspot/src/share/vm/opto/idealKit.cpp Mon Sep 14 09:49:54 2009 -0700
@@ -378,7 +378,7 @@
// Card mark store. Must be ordered so that it will come after the store of
// the oop.
-Node* IdealKit::storeCM(Node* ctl, Node* adr, Node *val, Node* oop_store,
+Node* IdealKit::storeCM(Node* ctl, Node* adr, Node *val, Node* oop_store, int oop_adr_idx,
BasicType bt,
int adr_idx) {
assert(adr_idx != Compile::AliasIdxTop, "use other store_to_memory factory" );
@@ -388,7 +388,7 @@
// Add required edge to oop_store, optimizer does not support precedence edges.
// Convert required edge to precedence edge before allocation.
- Node* st = new (C, 5) StoreCMNode(ctl, mem, adr, adr_type, val, oop_store);
+ Node* st = new (C, 5) StoreCMNode(ctl, mem, adr, adr_type, val, oop_store, oop_adr_idx);
st = transform(st);
set_memory(st, adr_idx);
--- a/hotspot/src/share/vm/opto/idealKit.hpp Thu Sep 10 18:18:06 2009 -0700
+++ b/hotspot/src/share/vm/opto/idealKit.hpp Mon Sep 14 09:49:54 2009 -0700
@@ -216,6 +216,7 @@
Node* adr,
Node* val,
Node* oop_store,
+ int oop_adr_idx,
BasicType bt,
int adr_idx);
--- a/hotspot/src/share/vm/opto/memnode.cpp Thu Sep 10 18:18:06 2009 -0700
+++ b/hotspot/src/share/vm/opto/memnode.cpp Mon Sep 14 09:49:54 2009 -0700
@@ -2313,6 +2313,22 @@
return this;
}
+//=============================================================================
+//------------------------------Ideal---------------------------------------
+Node *StoreCMNode::Ideal(PhaseGVN *phase, bool can_reshape){
+ Node* progress = StoreNode::Ideal(phase, can_reshape);
+ if (progress != NULL) return progress;
+
+ Node* my_store = in(MemNode::OopStore);
+ if (my_store->is_MergeMem()) {
+ Node* mem = my_store->as_MergeMem()->memory_at(oop_alias_idx());
+ set_req(MemNode::OopStore, mem);
+ return this;
+ }
+
+ return NULL;
+}
+
//------------------------------Value-----------------------------------------
const Type *StoreCMNode::Value( PhaseTransform *phase ) const {
// Either input is TOP ==> the result is TOP
--- a/hotspot/src/share/vm/opto/memnode.hpp Thu Sep 10 18:18:06 2009 -0700
+++ b/hotspot/src/share/vm/opto/memnode.hpp Mon Sep 14 09:49:54 2009 -0700
@@ -582,12 +582,16 @@
// The last StoreCM before a SafePoint must be preserved and occur after its "oop" store
// Preceeding equivalent StoreCMs may be eliminated.
class StoreCMNode : public StoreNode {
+ private:
+ int _oop_alias_idx; // The alias_idx of OopStore
public:
- StoreCMNode( Node *c, Node *mem, Node *adr, const TypePtr* at, Node *val, Node *oop_store ) : StoreNode(c,mem,adr,at,val,oop_store) {}
+ StoreCMNode( Node *c, Node *mem, Node *adr, const TypePtr* at, Node *val, Node *oop_store, int oop_alias_idx ) : StoreNode(c,mem,adr,at,val,oop_store), _oop_alias_idx(oop_alias_idx) {}
virtual int Opcode() const;
virtual Node *Identity( PhaseTransform *phase );
+ virtual Node *Ideal(PhaseGVN *phase, bool can_reshape);
virtual const Type *Value( PhaseTransform *phase ) const;
virtual BasicType memory_type() const { return T_VOID; } // unspecific
+ int oop_alias_idx() const { return _oop_alias_idx; }
};
//------------------------------LoadPLockedNode---------------------------------
--- a/hotspot/src/share/vm/opto/superword.cpp Thu Sep 10 18:18:06 2009 -0700
+++ b/hotspot/src/share/vm/opto/superword.cpp Mon Sep 14 09:49:54 2009 -0700
@@ -457,10 +457,6 @@
} else if (out->Opcode() == Op_StoreCM && out->in(MemNode::OopStore) == n) {
// StoreCM has an input edge used as a precedence edge.
// Maybe an issue when oop stores are vectorized.
- } else if( out->is_MergeMem() && prev &&
- prev->Opcode() == Op_StoreCM && out == prev->in(MemNode::OopStore)) {
- // Oop store is a MergeMem! This should not happen. Temporarily remove the assertion
- // for this case because it could not be superwordized anyway.
} else {
assert(out == prev || prev == NULL, "no branches off of store slice");
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/6877254/Test.java Mon Sep 14 09:49:54 2009 -0700
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2009 Sun Microsystems, Inc. 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/**
+ * @test
+ * @bug 6877254
+ * @summary Implement StoreCMNode::Ideal to promote its OopStore above the MergeMem
+ *
+ * @run main/othervm -server -Xcomp -XX:+UseConcMarkSweepGC Test
+ */
+
+public class Test {
+ static byte var_1;
+ static String var_2 = "";
+ static byte var_3;
+ static float var_4 = 0;
+
+ public static void main(String[] args) {
+ int i = 0;
+
+ for (String var_tmp = var_2; i < 11; var_1 = 0, i++) {
+ var_2 = var_2;
+ var_4 *= (var_4 *= (var_3 = 0));
+ }
+
+ System.out.println("var_1 = " + var_1);
+ System.out.println("var_2 = " + var_2);
+ System.out.println("var_3 = " + var_3);
+ System.out.println("var_4 = " + var_4);
+ }
+}