6877254: Server vm crashes with no branches off of store slice" when run with CMS and UseSuperWord(default)
authorcfang
Mon, 14 Sep 2009 09:49:54 -0700
changeset 3904 007a45522a7f
parent 3805 3cddd4882151
child 3905 7d725029ac85
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
hotspot/src/share/vm/opto/graphKit.cpp
hotspot/src/share/vm/opto/graphKit.hpp
hotspot/src/share/vm/opto/idealKit.cpp
hotspot/src/share/vm/opto/idealKit.hpp
hotspot/src/share/vm/opto/memnode.cpp
hotspot/src/share/vm/opto/memnode.hpp
hotspot/src/share/vm/opto/superword.cpp
hotspot/test/compiler/6877254/Test.java
--- 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);
+    }
+}