changeset 26180:2fbed11af70e

8055153: nsk/stress/jck60/jck60014 crashes on sparc Summary: missing control for LoadRange and LoadKlass nodes created during arraycopy node expansion Reviewed-by: kvn, iveresov
author roland
date Tue, 19 Aug 2014 17:23:38 +0200
parents b6f05d95bd72
children b8d61f186627
files hotspot/src/share/vm/opto/callnode.cpp hotspot/src/share/vm/opto/callnode.hpp hotspot/src/share/vm/opto/library_call.cpp hotspot/src/share/vm/opto/macro.cpp hotspot/src/share/vm/opto/macroArrayCopy.cpp hotspot/test/compiler/arraycopy/TestMissingControl.java
diffstat 6 files changed, 152 insertions(+), 32 deletions(-) [+]
line wrap: on
line diff
--- a/hotspot/src/share/vm/opto/callnode.cpp	Tue Aug 19 16:20:18 2014 -0700
+++ b/hotspot/src/share/vm/opto/callnode.cpp	Tue Aug 19 17:23:38 2014 +0200
@@ -1826,17 +1826,25 @@
 uint ArrayCopyNode::size_of() const { return sizeof(*this); }
 
 ArrayCopyNode* ArrayCopyNode::make(GraphKit* kit, bool may_throw,
-                                   Node* src, Node* src_offset, Node* dest, Node* dest_offset, Node* length,
-                                   bool alloc_tightly_coupled) {
+                                   Node* src, Node* src_offset,
+                                   Node* dest, Node* dest_offset,
+                                   Node* length,
+                                   bool alloc_tightly_coupled,
+                                   Node* src_length, Node* dest_length,
+                                   Node* src_klass, Node* dest_klass) {
 
   ArrayCopyNode* ac = new ArrayCopyNode(kit->C, alloc_tightly_coupled);
   Node* prev_mem = kit->set_predefined_input_for_runtime_call(ac);
 
-  ac->init_req( ArrayCopyNode::Src, src);
-  ac->init_req( ArrayCopyNode::SrcPos, src_offset);
-  ac->init_req( ArrayCopyNode::Dest, dest);
-  ac->init_req( ArrayCopyNode::DestPos, dest_offset);
-  ac->init_req( ArrayCopyNode::Length, length);
+  ac->init_req(ArrayCopyNode::Src, src);
+  ac->init_req(ArrayCopyNode::SrcPos, src_offset);
+  ac->init_req(ArrayCopyNode::Dest, dest);
+  ac->init_req(ArrayCopyNode::DestPos, dest_offset);
+  ac->init_req(ArrayCopyNode::Length, length);
+  ac->init_req(ArrayCopyNode::SrcLen, src_length);
+  ac->init_req(ArrayCopyNode::DestLen, dest_length);
+  ac->init_req(ArrayCopyNode::SrcKlass, src_klass);
+  ac->init_req(ArrayCopyNode::DestKlass, dest_klass);
 
   if (may_throw) {
     ac->set_req(TypeFunc::I_O , kit->i_o());
--- a/hotspot/src/share/vm/opto/callnode.hpp	Tue Aug 19 16:20:18 2014 -0700
+++ b/hotspot/src/share/vm/opto/callnode.hpp	Tue Aug 19 17:23:38 2014 +0200
@@ -1097,11 +1097,15 @@
 
   static const TypeFunc* arraycopy_type() {
     const Type** fields = TypeTuple::fields(ParmLimit - TypeFunc::Parms);
-    fields[Src]     = TypeInstPtr::BOTTOM;
-    fields[SrcPos]  = TypeInt::INT;
-    fields[Dest]    = TypeInstPtr::BOTTOM;
-    fields[DestPos] = TypeInt::INT;
-    fields[Length]  = TypeInt::INT;
+    fields[Src]       = TypeInstPtr::BOTTOM;
+    fields[SrcPos]    = TypeInt::INT;
+    fields[Dest]      = TypeInstPtr::BOTTOM;
+    fields[DestPos]   = TypeInt::INT;
+    fields[Length]    = TypeInt::INT;
+    fields[SrcLen]    = TypeInt::INT;
+    fields[DestLen]   = TypeInt::INT;
+    fields[SrcKlass]  = TypeKlassPtr::BOTTOM;
+    fields[DestKlass] = TypeKlassPtr::BOTTOM;
     const TypeTuple *domain = TypeTuple::make(ParmLimit, fields);
 
     // create result type (range)
@@ -1122,12 +1126,20 @@
     Dest,
     DestPos,
     Length,
+    SrcLen,
+    DestLen,
+    SrcKlass,
+    DestKlass,
     ParmLimit
   };
 
   static ArrayCopyNode* make(GraphKit* kit, bool may_throw,
-                             Node* src, Node* src_offset, Node* dest, Node* dest_offset, Node* length,
-                             bool alloc_tightly_coupled);
+                             Node* src, Node* src_offset,
+                             Node* dest,  Node* dest_offset,
+                             Node* length,
+                             bool alloc_tightly_coupled,
+                             Node* src_length = NULL, Node* dest_length = NULL,
+                             Node* src_klass = NULL, Node* dest_klass = NULL);
 
   void connect_outputs(GraphKit* kit);
 
--- a/hotspot/src/share/vm/opto/library_call.cpp	Tue Aug 19 16:20:18 2014 -0700
+++ b/hotspot/src/share/vm/opto/library_call.cpp	Tue Aug 19 17:23:38 2014 +0200
@@ -4788,7 +4788,12 @@
   }
 
   AllocateArrayNode* alloc = tightly_coupled_allocation(dest, NULL);
-  ArrayCopyNode* ac = ArrayCopyNode::make(this, true, src, src_offset, dest, dest_offset, length, alloc != NULL);
+  ArrayCopyNode* ac = ArrayCopyNode::make(this, true, src, src_offset, dest, dest_offset, length, alloc != NULL,
+                                          // Create LoadRange and LoadKlass nodes for use during macro expansion here
+                                          // so the compiler has a chance to eliminate them: during macro expansion,
+                                          // we have to set their control (CastPP nodes are eliminated).
+                                          load_array_length(src), load_array_length(dest),
+                                          load_object_klass(src), load_object_klass(dest));
 
   if (notest) {
     ac->set_arraycopy_notest();
--- a/hotspot/src/share/vm/opto/macro.cpp	Tue Aug 19 16:20:18 2014 -0700
+++ b/hotspot/src/share/vm/opto/macro.cpp	Tue Aug 19 17:23:38 2014 +0200
@@ -94,7 +94,8 @@
     newcall->add_req(old_in);
   }
 
-  newcall->set_jvms(oldcall->jvms());
+  // JVMS may be shared so clone it before we modify it
+  newcall->set_jvms(oldcall->jvms() != NULL ? oldcall->jvms()->clone_deep(C) : NULL);
   for (JVMState *jvms = newcall->jvms(); jvms != NULL; jvms = jvms->caller()) {
     jvms->set_map(newcall);
     jvms->set_locoff(jvms->locoff()+jvms_adj);
--- a/hotspot/src/share/vm/opto/macroArrayCopy.cpp	Tue Aug 19 16:20:18 2014 -0700
+++ b/hotspot/src/share/vm/opto/macroArrayCopy.cpp	Tue Aug 19 17:23:38 2014 +0200
@@ -503,14 +503,8 @@
     // (Actually, we don't move raw bits only; the GC requires card marks.)
 
     // Get the klass* for both src and dest
-    Node* k_adr =  new AddPNode(src, src, MakeConX(oopDesc::klass_offset_in_bytes()));
-    transform_later(k_adr);
-    Node* src_klass  = LoadKlassNode::make(_igvn, C->immutable_memory(), k_adr, TypeInstPtr::KLASS);
-    transform_later(src_klass);
-    k_adr =  new AddPNode(dest, dest, MakeConX(oopDesc::klass_offset_in_bytes()));
-    transform_later(k_adr);
-    Node* dest_klass  = LoadKlassNode::make(_igvn, C->immutable_memory(), k_adr, TypeInstPtr::KLASS);
-    transform_later(dest_klass);
+    Node* src_klass  = ac->in(ArrayCopyNode::SrcKlass);
+    Node* dest_klass = ac->in(ArrayCopyNode::DestKlass);
 
     // Generate the subtype check.
     // This might fold up statically, or then again it might not.
@@ -1214,20 +1208,14 @@
     // generate_negative_guard(length, slow_region);
 
     // (7) src_offset + length must not exceed length of src.
-    Node* r_adr =  new AddPNode(src, src, MakeConX(arrayOopDesc::length_offset_in_bytes()));
-    transform_later(r_adr);
-    Node* alen = new LoadRangeNode(0, C->immutable_memory(), r_adr, TypeInt::POS);
-    transform_later(alen);
+    Node* alen = ac->in(ArrayCopyNode::SrcLen);
     generate_limit_guard(&ctrl,
                          src_offset, length,
                          alen,
                          slow_region);
 
     // (8) dest_offset + length must not exceed length of dest.
-    r_adr =  new AddPNode(dest, dest, MakeConX(arrayOopDesc::length_offset_in_bytes()));
-    transform_later(r_adr);
-    alen = new LoadRangeNode(0, C->immutable_memory(), r_adr, TypeInt::POS);
-    transform_later(alen);
+    alen = ac->in(ArrayCopyNode::DestLen);
     generate_limit_guard(&ctrl,
                          dest_offset, length,
                          alen,
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/arraycopy/TestMissingControl.java	Tue Aug 19 17:23:38 2014 +0200
@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2014, 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
+ * 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 Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8055153
+ * @summary missing control on LoadRange and LoadKlass when array copy macro node is expanded
+ * @run main/othervm -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:-TieredCompilation TestMissingControl
+ *
+ */
+public class TestMissingControl {
+
+    static int[] m1(int[] a2) {
+        int[] a1 = new int[10];
+        System.arraycopy(a1, 0, a2, 0, 10);
+        return a1;
+    }
+
+    static class A {
+    }
+
+    static Object m2(Object[] a2) {
+        A[] a1 = new A[10];
+        System.arraycopy(a1, 0, a2, 0, 10);
+        return a1;
+    }
+
+    static void test1() {
+        int[] a2 = new int[10];
+        int[] a3 = new int[5];
+
+        // compile m1 with arraycopy intrinsified
+        for (int i = 0; i < 20000; i++) {
+            m1(a2);
+        }
+
+        // make m1 trap
+        for (int i = 0; i < 10; i++) {
+            try {
+                m1(a3);
+            } catch(IndexOutOfBoundsException ioobe) {
+            }
+        }
+
+        // recompile m1
+        for (int i = 0; i < 20000; i++) {
+            m1(a2);
+        }
+
+        try {
+            m1(null);
+        } catch(NullPointerException npe) {}
+    }
+
+    static void test2() {
+        A[] a2 = new A[10];
+        A[] a3 = new A[5];
+
+        // compile m2 with arraycopy intrinsified
+        for (int i = 0; i < 20000; i++) {
+            m2(a2);
+        }
+
+        // make m2 trap
+        for (int i = 0; i < 10; i++) {
+            try {
+                m2(a3);
+            } catch(IndexOutOfBoundsException ioobe) {
+            }
+        }
+
+        // recompile m2
+        for (int i = 0; i < 20000; i++) {
+            m2(a2);
+        }
+
+        try {
+            m2(null);
+        } catch(NullPointerException npe) {}
+    }
+
+    static public void main(String[] args) {
+        test1();
+        test2();
+    }
+}